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

           Summary: Members of non-lvalues can be assigned to.
           Product: D
           Version: 2.030
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: chadjoan@gmail.com


struct S { int a = 0; }
S foo() { S s; return s; }

void main()
{
    foo.a++;
    foo().a++;
    foo.a = 42;
}

This compiles but is nonsensical.  It is impossible to modify foo.a, yet the compiler lets you try anyway.

Tried it with DMD 2.030 on Linux.

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


BCS <shro8822@vandals.uidaho.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |shro8822@vandals.uidaho.edu




--- Comment #1 from BCS <shro8822@vandals.uidaho.edu>  2009-05-18 21:20:21 PDT ---
The problem is not that ++/+= work on lvalues (they don't). The problem is that the members of a struct returned from a function are lvalues.

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


Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com




--- Comment #2 from Steven Schveighoffer <schveiguy@yahoo.com>  2009-05-19 07:01:51 PDT ---
Consider this actually sensical case:

struct S
{
  int *a;
}

S foo() {...}

*foo.a = 3;

Clearly, you don't want structs to be non-modifyiable in all cases.  If S has member functions which modify both values of S and values S references, then you should able to call those functions also.  It might be very difficult for the compiler to distinguish all these cases.

I'd say the case you give is non-sensical, but still valid code.

This is also non-sensical code, in the same light, and I don't think it should be a compiler error:

void foo()
{
   int x;
}

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


Jarrett Billingsley <jarrett.billingsley@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jarrett.billingsley@gmail.c
                   |                            |om




--- Comment #3 from Jarrett Billingsley <jarrett.billingsley@gmail.com>  2009-07-28 18:09:12 PDT ---
(In reply to comment #2)
> Consider this actually sensical case:
> 
> struct S
> {
>   int *a;
> }
> 
> S foo() {...}
> 
> *foo.a = 3;
> 
> Clearly, you don't want structs to be non-modifyiable in all cases.  If S has member functions which modify both values of S and values S references, then you should able to call those functions also.  It might be very difficult for the compiler to distinguish all these cases.

Actually the compiler wouldn't have to do anything special here.  If the return of foo() were an rvalue, then *foo().a could still be an lvalue, not because of any special rules, but because of how the dereference operator works.

A more thorough explanation: DMD will diagnose no-op statements such as:

x + y;

Since such code is dead and possibly a typo (say you accidentally shifted your assignment operator there).  Code like:

struct S { int a; }
S foo() { return S(5); }
foo().a++;

Should be nonsensical for the same reason.  foo() is an rvalue, and
rvalue.field yields an rvalue.  Similarly, in your example, "foo().a = new
int;" would be nonsensical, since again, rvalue.field yields an rvalue.  But as
soon as you use the dereference operator, foo().a becomes an lvalue.  (In fact
I think K&R describe * and & in such terms (turning lvalues into rvalues and
vice versa).)

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





--- Comment #4 from Steven Schveighoffer <schveiguy@yahoo.com>  2009-07-29 10:11:26 PDT ---
(In reply to comment #3)
> Actually the compiler wouldn't have to do anything special here.  If the return of foo() were an rvalue, then *foo().a could still be an lvalue, not because of any special rules, but because of how the dereference operator works.

Yes, that would be helpful.

I think you are right that it can be determined in simple cases, but for sure there will be cases that the compiler cannot diagnose, such as:

int _global;

struct S
{
  int _x;
  version(noop)
    void x(int n) { _x = n;}
  else
    void x(int n) { _global = n;}
}

struct S2
{
  S foo() { return S(5);}
}

void main()
{
  S2 s2;
  s2.foo.x = 5;
}

How does the compiler know when compiling with noop that the s2.foo.x = 5 doesn't do anything?  Especially if the module containing main is using a di file to define S and S2.

The result is, I don't think the compiler can diganose the complex cases, and most of the time, the cases are complex.

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





--- Comment #5 from Chad Joan <chadjoan@gmail.com>  2009-07-30 01:50:28 PDT ---
(In reply to comment #4)
> 
> I think you are right that it can be determined in simple cases, but for sure there will be cases that the compiler cannot diagnose, such as:
> 
> int _global;
> 
> struct S
> {
>   int _x;
>   version(noop)
>     void x(int n) { _x = n;}
>   else
>     void x(int n) { _global = n;}
> }
> 
> struct S2
> {
>   S foo() { return S(5);}
> }
> 
> void main()
> {
>   S2 s2;
>   s2.foo.x = 5;
> }
> 
> How does the compiler know when compiling with noop that the s2.foo.x = 5 doesn't do anything?  Especially if the module containing main is using a di file to define S and S2.
> 
> The result is, I don't think the compiler can diganose the complex cases, and most of the time, the cases are complex.

It's easy for the compiler to know that "s2.foo.x = 5" does nothing.  When compiling with noop, the "void x(int n) { _global = n;}" version just does not get compiled.  Period.  When "s2.foo.x = 5;" is being analysed, the compiler will walk the syntax tree for "void x(int n) { _x = n;}" and discover that s2.foo is an rvalue.  This is what it already does, minus the "discover that s2.foo is an rvalue" part.

Of course, s2.foo().x(5) violates the principle at play here.  At this point, the whole "version(noop)" thing is just fluff, and here is the meat of the matter.  In this example, "s2.foo.x = 5;" does actually do something and is reasonable code.  However, naively forbidding an rvalue on the lhs of an assign expression will make that code fail to compile.  I don't feel that code like this is terribly common or that much better than the alternatives, so it is probably worth losing some corner cases like these for the sake of preventing nasty bugs.

.di files change absolutely nothing.  They are .d files that just happen to be mostly definitions because dmd generated that way.  There is nothing in the spec saying that they even need to exist.  More importantly, Walter was intentional about omitting them from the spec.

"D interface files bear some analogous similarities to C++ header files. But they are not required in the way that C++ header files are, and they are not part of the D language. They are a feature of the compiler, and serve only as an optimization of the build process." http://www.digitalmars.com/d/2.0/dmd-linux.html#interface_files

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





--- Comment #6 from Chad Joan <chadjoan@gmail.com>  2009-07-30 02:09:09 PDT ---
On the newsgroup KennyTM~ pointed out that opDot() also suffers from this
problem:

struct S {
  int s;
}

class X {
  S opDot() { S temp; temp.s = 6; return temp; }
}

X z = new X;
assert(z.s == 6);
z.s = 3;
assert(z.s == 6);

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





--- Comment #7 from Steven Schveighoffer <schveiguy@yahoo.com>  2009-07-30 07:19:24 PDT ---
(In reply to comment #5)

> Of course, s2.foo().x(5) violates the principle at play here.  At this point, the whole "version(noop)" thing is just fluff, and here is the meat of the matter.  In this example, "s2.foo.x = 5;" does actually do something and is reasonable code.  However, naively forbidding an rvalue on the lhs of an assign expression will make that code fail to compile.  I don't feel that code like this is terribly common or that much better than the alternatives, so it is probably worth losing some corner cases like these for the sake of preventing nasty bugs.

You are killing the entire feature of making user-defined "builtin" types, such as a custom pointer type.  Since those would undoubtedly be structs, and therefore returned as rvalues, you could not use them for anything without first creating lvalues out of them.  If we are to have such constructs, they should be on par with native pointers.  At the very least, we should have a way to mark such structs as "allow rvalue operations."  I would be ok with that.

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





--- Comment #8 from BCS <shro8822@vandals.uidaho.edu>  2009-07-30 09:23:10 PDT ---
(In reply to comment #5)
> 
> It's easy for the compiler to know that "s2.foo.x = 5" does nothing.  When compiling with noop, the "void x(int n) { _global = n;}" version just does not get compiled.  Period.  When "s2.foo.x = 5;" is being analysed, the compiler will walk the syntax tree for "void x(int n) { _x = n;}" and discover that s2.foo is an rvalue.  This is what it already does, minus the "discover that s2.foo is an rvalue" part.

This assumes that the body of x is available for analysis and is simple enough to be analysed.

The first will not be true of any C function, any function that calls a C
function (etc.), any closed source lib or any function that calls into a closed
source lib (etc.).

The second will not be true in general because it can devolve into the halting problem.

To keep things consistent, the rules used have to be defined in the spec, practicely ruling out powerful heuristics and computation logic systems, and have to act the same regardless of accessability to source, strongly ruling out anything that uses inter-procedural analysis (Walter has several times stated that semantic rules that require analysis of non local code are not going to happen).

> .di files change absolutely nothing.

Technically, yes. .di files don't actually /add/ any problems here because the problems at issue can be caused by a .d file as well.

> They are .d files that just happen to be
> mostly definitions because dmd generated that way.  There is nothing in the
> spec saying that they even need to exist.

Any implementation that doesn't give the same functionality as .di files (no-source symbol decelerations) effectively eliminates the ability to have closed source D libraries. Therefor, the feature causing problems effectively is part of the spec.

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





--- Comment #9 from Chad Joan <chadjoan@gmail.com>  2009-07-30 10:38:44 PDT ---
(In reply to comment #7)
> (In reply to comment #5)
> 
> > Of course, s2.foo().x(5) violates the principle at play here.  At this point, the whole "version(noop)" thing is just fluff, and here is the meat of the matter.  In this example, "s2.foo.x = 5;" does actually do something and is reasonable code.  However, naively forbidding an rvalue on the lhs of an assign expression will make that code fail to compile.  I don't feel that code like this is terribly common or that much better than the alternatives, so it is probably worth losing some corner cases like these for the sake of preventing nasty bugs.
> 
> You are killing the entire feature of making user-defined "builtin" types, such as a custom pointer type.  Since those would undoubtedly be structs, and therefore returned as rvalues, you could not use them for anything without first creating lvalues out of them.  If we are to have such constructs, they should be on par with native pointers.  At the very least, we should have a way to mark such structs as "allow rvalue operations."  I would be ok with that.

You make it sound like we wouldn't be able to use structs anymore!

Not the case.

struct S
{
  int _x;
  version(noop)
    void x(int n) { _x = n;}
  else
    void x(int n) { _global = n;}
}

struct S2
{
  S foo() { return S(5);}
}

void main()
{
    S2 s2;

    s2.foo.x = 5; // not good.

    int bar = s2.foo.x + 42; // fine, rvalue is not mutated, only read.

    auto s = s2.foo;
    s.x = 5; // fine

    S s1;
    s1.x = 5; // fine
    s1._x; // fine, it was declared public.
}

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