Bug 5117 – [CTFE] Member function call with rather complex this: side effects ignored

The interpretor neglects member functions mutating 'this' if a function call involves two or more chained dot expressions. In the following repro, s.change() succeeds in mutating 'this' but r.s.change() does not: -------------------- static int dummy = test(); int test() { S s; s.change(); assert(s.value == 1); // (7) succeeds R r; r.s.change(); assert(r.s.value == 1); // (11) fails, value == 0 return 0; } struct S { int value; void change() { value = 1; } } struct R { S s; } -------------------- % dmd -o- -c test.d test.d(11): Error: assert(r.s.value == 1) failed test.d(1): Error: cannot evaluate test() at compile time test.d(1): Error: cannot evaluate test() at compile time --------------------

The problem lies in FuncDeclralation::interpret(), around line 223: -------------------- // Don't restore the value of 'this' upon function return if (needThis() && thisarg->op == TOKvar && istate) { VarDeclaration *thisvar = ((VarExp *)(thisarg))->var->isVarDeclaration(); for (size_t i = 0; i < istate->vars.dim; i++) { VarDeclaration *v = (VarDeclaration *)istate->vars.data[i]; if (v == thisvar) { istate->vars.data[i] = NULL; break; } } } -------------------- In the repro code in comment #1, thisarg is 'r.s' (TOKdotvar) and the local variable 'r' is not removed from the "restore list" istate->vars. Then, the interpretor wrongly restores 'r' to init. Just dealing with TOKdotvar fixes the specific reported problem, but it's not a general fix. Still the interpretor should deal with references. For example: -------------------- enum dummy = test(); int test() { S s; getRef(s).change(); assert(s.value == 1); // fails, value == 0 return 0; } ref S getRef(ref S s) { return s; } struct S { int value; void change() { value = 1; } } --------------------

PATCH: interpret.c, line 224, FuncDeclaration::interpret(). // Don't restore the value of 'this' upon function return - if (needThis() && thisarg->op == TOKvar && istate) + if (needThis() && istate) { - VarDeclaration *thisvar = ((VarExp *)(thisarg))->var->isVarDeclaration(); + VarDeclaration *thisvar = findParentVar(thisarg, istate->localThis); for (size_t i = 0; i < istate->vars.dim; i++) { VarDeclaration *v = (VarDeclaration *)istate->vars.data[i]; if (v == thisvar) { istate->vars.data[i] = NULL; break; } and also add VarDeclaration * findParentVar(Expression *e, Expression *thisval); to the top of the file. This fixes both test cases.

I applied the patch, and the first test case now works but the second still fails: test.d(7): Error: assert(s.value == 1) failed test.d(1): Error: cannot evaluate test() at compile time test.d(1): Error: cannot evaluate test() at compile time Perhaps your sources differ in other ways? http://www.dsource.org/projects/dmd/changeset/742

(In reply to comment #3) > I applied the patch, and the first test case now works but the second still > fails: > > test.d(7): Error: assert(s.value == 1) failed > test.d(1): Error: cannot evaluate test() at compile time > test.d(1): Error: cannot evaluate test() at compile time > > Perhaps your sources differ in other ways? > > http://www.dsource.org/projects/dmd/changeset/742 Not sure what's happened here. Maybe I got the test case wrong. Regardless, this change (line 228) fixes it. // Don't restore the value of 'this' upon function return if (needThis() && istate) { VarDeclaration *thisvar = findParentVar(thisarg, istate->localThis); + if (!thisvar) // it's a reference. Find which variable it refers to. + thisvar = findParentVar(thisarg->interpret(istate), istate->localThis); for (size_t i = 0; i < istate->vars.dim; i++) { VarDeclaration *v = (VarDeclaration *)istate->vars.data[i];

That did the trick, thanks! http://www.dsource.org/projects/dmd/changeset/747