http://d.puremagic.com/issues/show_bug.cgi?id=7603

           Summary: Default initializer is allowed on ref/out params
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: accepts-invalid
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: andrej.mitrovich@gmail.com


--- Comment #0 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-02-27 18:44:34 PST ---
void test1(ref bool val = true) {
}

void test2(out bool val = true) {
}

void main()
{
    bool x;
    test1(x);  // compiles ok
    test2(x);  // compiles ok

    test1();  // Error: constant true is not an lvalue
    test2();  // Error: constant true is not an lvalue
}

The compiler should probably disallow that kind of function signature regardless of any calls. The initializer doesn't really do anything.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603


bearophile_hugs@eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs@eml.cc


--- Comment #1 from bearophile_hugs@eml.cc 2012-02-27 19:56:57 PST ---
Ow.

Even this fails:

void test1(ref int val = 10) {}
void test2(out int val = 20) {}
void main() {
    int x;
    test1(x);
    assert(x == 10);
    test2(x);
    assert(x == 20);
}

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603



--- Comment #2 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-04 10:34:29 PDT ---
(In reply to comment #0)
> void test1(ref bool val = true) {
> }
> 
> void test2(out bool val = true) {
> }

Actually I'm only half-right. Phobos uses this in std.random:

void randomShuffle(Range, RandomGen = Random)(Range r, ref RandomGen gen =
rndGen);

So 'val' is either a reference to a passed-in argument, or a reference to the default argument which is an lvalue.

Default argument for 'ref' parameter should only be allowed if the argument is an lvalue ('true' is an rvalue'). The compiler already checks this but *only* if the function is called without arguments:

void test(ref bool val = true) { }

void main()
{
    bool b;
    test(b); // ok, no errors
    test();  // errors only on this invocation
}

The compiler should error as soon as it sees the invalid declaration. I'll make a pull shortly.

Btw, the same applies to 'out'. An rvalue isn't valid, but an lvalue as default argument is perfectly valid:

bool w = true;
void test(out bool val = w) { }

void main()
{
    test();
    assert(w == false);
}

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603



--- Comment #3 from bearophile_hugs@eml.cc 2012-10-04 12:27:15 PDT ---
See also Issue 5850

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603


Maxim Fomin <maxim@maxim-fomin.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |maxim@maxim-fomin.ru


--- Comment #4 from Maxim Fomin <maxim@maxim-fomin.ru> 2012-10-04 21:32:15 PDT ---
(In reply to comment #1)
> Ow.
> 
> Even this fails:
> 
> void test1(ref int val = 10) {}
> void test2(out int val = 20) {}
> void main() {
>     int x;
>     test1(x);
>     assert(x == 10);
>     test2(x);
>     assert(x == 20);
> }

Why wouldn't this fail? Default arguments are used if no argument is given. Sine you provide arguments and functions don't modify them, arguments are not changed. The only modification happens due to parameter storage class out.

 void test1(ref int val = 10) {}
 void test2(out int val = 20) {}
 void main() {
     int x = 5;
     test1(x);
     assert(x == 5);
     test2(x);
     assert(x == 0);
}

Passes both assertions as it should.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603



--- Comment #5 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-05 09:30:41 PDT ---
(In reply to comment #4)
> Why wouldn't this fail? Default arguments are used if no argument is given. Sine you provide arguments and functions don't modify them, arguments are not changed. The only modification happens due to parameter storage class out.
> 
>  void test1(ref int val = 10) {}
>  void test2(out int val = 20) {}
>  void main() {
>      int x = 5;
>      test1(x);
>      assert(x == 5);
>      test2(x);
>      assert(x == 0);
> }
> 
> Passes both assertions as it should.

Ignoring what bear's confusion here, the function declaration is invalid. Call
test1() alone:

void test1(ref int val = 10) { }
void main()
{
    test1();  // Error: constant 10 is not an lvalue
}

It makes no sense adding default arguments if they'll never compile, so this needs to be a CT error when the function is parsed and not when the function is called.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603



--- Comment #6 from Maxim Fomin <maxim@maxim-fomin.ru> 2012-10-05 09:41:09 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> > Why wouldn't this fail? Default arguments are used if no argument is given. Sine you provide arguments and functions don't modify them, arguments are not changed. The only modification happens due to parameter storage class out.
> > 
> >  void test1(ref int val = 10) {}
> >  void test2(out int val = 20) {}
> >  void main() {
> >      int x = 5;
> >      test1(x);
> >      assert(x == 5);
> >      test2(x);
> >      assert(x == 0);
> > }
> > 
> > Passes both assertions as it should.
> 
> Ignoring what bear's confusion here, the function declaration is invalid. Call
> test1() alone:
> 
> void test1(ref int val = 10) { }
> void main()
> {
>     test1();  // Error: constant 10 is not an lvalue
> }
> 
> It makes no sense adding default arguments if they'll never compile, so this needs to be a CT error when the function is parsed and not when the function is called.

Agree here, but I was talking about that it is logically that those asserts should fail, rather than about accepting non-lvalue default arguments.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603



--- Comment #7 from github-bugzilla@puremagic.com 2012-10-06 11:40:42 PDT ---
Commit pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/67681b916f7880432c7da14a714404af0254278e Merge pull request #1163 from AndrejMitrovic/Fix7603

Fix Issue 7603 - Default initializer for ref/out must be an lvalue

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

http://d.puremagic.com/issues/show_bug.cgi?id=7603


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |yebblies@gmail.com
         Resolution|                            |FIXED


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

http://d.puremagic.com/issues/show_bug.cgi?id=7603



--- Comment #8 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-07 14:04:47 PDT ---
I've just realized I shot myself in the foot with this change (well,
partially).

Direct snippet of some code I've used (the main thing to look for is "ref Set!SymID oldIDs = Set!SymID.init")

    SymID[] getClassTreeImpl(Table)(Class par, Table table, ref Set!SymID
oldIDs = Set!SymID.init, bool skipRoot = false)
    {
        typeof(return) result;

        if (!skipRoot && par.ID !in oldIDs)
        {
            oldIDs.add(par.ID);
            result ~= par.ID;
        }

        foreach (ID, cls; table)
        {
            if (cls.baseIDs.canFind(par.ID))
                result ~= getClassTreeImpl(cls, table, oldIDs);
        }

        return result;
    }

Basically, getClassTreeImpl can be passed an existing set of keys or it will create a new one if not provided and will update it during recursive calls.

I can fix my code, but I worry if my changeset is going to brake anyone elses code. I wonder if this trick is used that often.. I guess we'll have to wait and see.

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