Invalid C++ code generation when returning var T

[BigEpsilon]

For the following example, the code that is generated is wrong and induce a segmentation fault.

Example

type 
    Vector*  {.importcpp: "std::vector", header: "vector".}[T] = object

proc initVector*[T](n: csize): Vector[T] 
    {.importcpp: "std::vector<'*0>(@)", header: "vector".}

proc unsafeIndex[T](this: var Vector[T], i: csize): var T 
    {.importcpp: "#[#]", header: "vector".}

proc `[]`*[T](this: var Vector[T], i: Natural): var T {.inline, noinit.} =
    when compileOption("boundChecks"):
        # this.checkIndex i
        discard
    result = this.unsafeIndex(i)

var v1 = initVector[int](10)
echo v1[0] # <-- SIGSEGV: Illegal storage access. (Attempt to read from nil?)

The error comes from the following generated lines:

static N_INLINE(NI*, X5BX5D__9bJ9bA39bRmdl9bxuHGDHoW1tQplay)(TY_DImjm4JODOV5hEJosD0PGw& this_0, NI i) {
	NI* result;
	nimfr_("[]", "play.nim");
	nimln_(16, "play.nim");
	result = this_0[((size_t) (i))]; // <-- should be:  result = &this_0[((size_t)(i))];
	popFrame();
	return result;
}

Current Output

Traceback (most recent call last)
play.nim(17)             play
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Expected Output

0

Workarounds

1. using this strange syntax fixes the bug:

proc `[]`*[T](this: var Vector[T], i: Natural): var T {.inline, noinit.} =
    when compileOption("boundChecks"):
        # this.checkIndex i
        discard
    # This strange syntax is to avoid a bug in the Nim c++ code generator
    result = (addr this.unsafeIndex(i))[]

Here is the code generated:

static N_INLINE(NI*, X5BX5D__9bJ9bA39bRmdl9bxuHGDHoW1tQplay)(TY_DImjm4JODOV5hEJosD0PGw& this_0, NI i) {
	NI* result;
	nimfr_("[]", "play.nim");
	nimln_(25, "play.nim");
	result = (&this_0[((size_t) (i))]); // <-- this is good
	popFrame();
	return result;
}

2. use a template instead of a proc

Additional Information

nim --version

Nim Compiler Version 0.19.9 [Linux: amd64]
Compiled at 2019-01-03
Copyright (c) 2006-2018 by Andreas Rumpf

active boot switches: -d:release

[cooldome]

IMO, var T doesn't map to cpp reference type, it is a pointer type instead. Therefore, what you need to do is:

proc unsafeIndex[T](this: var Vector[T], i: csize): var T 
    {.importcpp: "(&#[#])", header: "vector".}
``

[BigEpsilon]

Thank for your help. I tried using a pointer like you suggested because it seems to be the thing to do but the problem is that I get another invalid code generation error:

import strformat

type 
    Vector*  {.importcpp: "std::vector", header: "vector".}[T] = object

proc initVector*[T](n: csize): Vector[T] 
    {.importcpp: "std::vector<'*0>(@)", header: "vector".}

proc size*[T](this: Vector[T]): csize 
    {.noSideEffect, importcpp: "size", header: "vector".}

proc checkIndex[T](this: Vector[T], i: csize) : csize {.inline.} =
    if 0 > i or i > this.size:
        raise newException(IndexError, 
            &"index out of bounds: 0 <= (i:{i}) <= (n:{this.size})")
    result = i

proc unsafeIndex[T](this: var Vector[T], i: csize): var T 
    {.importcpp: "&#[#]", header: "vector".}

proc `[]`*[T](this: var Vector[T], i: Natural): var T {.inline, noinit.} =
    result = this.unsafeIndex(this.checkIndex i)

proc `[]=`*[T](this: var Vector[T], i: Natural, val: T) {.inline, noinit.} =
    this.unsafeIndex(this.checkIndex i) = val

var v1 = initVector[int](10)
v1[0] = 100 # <-- wrong code generated for this line
echo v1[0] 

which gives the following error:

/home/user/.cache/nim/play_d/play_play.cpp: In function ‘void X5BX5Deq__ZVQLhFsIfFuHeNr2UXhW2wplay(TY_DImjm4JODOV5hEJosD0PGw&, NI, NI)’:
/home/user/.cache/nim/play_d/play_play.cpp:443:74: error: lvalue required as left operand of assignment
  &this_0[checkIndex_USyPXguD9caC5SagGtT8zBAplay(this_0, ((size_t) (i)))] = val;

[Araq]

IMO, var T doesn't map to cpp reference type, it is a pointer type instead.

No, we try to map it to & but cannot always do it.

[Araq]

result = this_0[((size_t) (i))]; // <-- should be: result = &this_0[((size_t)(i))];

No, why should there be a &?

[BigEpsilon]

Because result if to type NI* in the generate code. Here the returned var T is mapped to a pointer T*. The segmentation fault comes from the fact that we assign this_0[0] (= 0 in my example) to a pointer then de-reference it.

[krux02]

I think we can und should map var T to a proper C++ reference.

[mratsim]

Might be related to #8053 as I also get a null pointer dereference with a var T result.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. If you think it is still a valid issue, write a comment below; otherwise it will be closed. Thank you for your contributions.

[jmgomez]

Shoudlnt this be closed after the test case?

[mratsim]

The test case hasn't been merged cc @ringabout