www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 5117] New: [CTFE] Member function call with chained dots: side effects ignored

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5117

           Summary: [CTFE] Member function call with chained dots: side
                    effects ignored
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: wrong-code
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: rsinfu gmail.com



---
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
--------------------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 25 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5117


Shin Fujishiro <rsinfu gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[CTFE] Member function call |[CTFE] Member function call
                   |with chained dots: side     |with rather complex this:
                   |effects ignored             |side effects ignored



---
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;
            }
        }
    }
--------------------


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; }
}
--------------------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 25 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5117


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
                 CC|                            |clugdbug yahoo.com.au
           Severity|normal                      |critical



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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 29 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5117


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla digitalmars.com



17:13:14 PST ---
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

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 07 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5117






 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];

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 08 2010
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5117


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED



13:38:48 PST ---
That did the trick, thanks!

http://www.dsource.org/projects/dmd/changeset/747

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 10 2010