On Wednesday, 9 January 2013 at 22:25:53 UTC, Jonathan M Davis wrote:
> On Wednesday, January 09, 2013 23:22:19 Andrej Mitrovic wrote:
>> On 1/9/13, Walter Bright <newshound2@digitalmars.com> wrote:
>> > It is not a bug.
>> 
>> Something related I want to ask you about:
>> 
>> struct S
>> {
>> int _m;
>> this(int m)
>> {
>> this._m = _m; // meant "this._m = m;"
>> }
>> }
>> 
>> I'd like to add a warning for identity assignments when it involves a
>> parameter and a field of the aggregate which is the parent of the
>> function. This was filed as
>> http://d.puremagic.com/issues/show_bug.cgi?id=4407.
>
> Why not just prevent identity assignments altogether? What would be the
> purpose of even allowing a variable to be assigned itself? I see no reason to
> single out parameters or member variables for that.
>
> - Jonathan M Davis

I can forsee someone somewhere having a problem with this if he wants to write a general enough templated solution. I actually can come up with an example.

Recently I wrote a template mixin witch works this way:
reassign(a, b, c, ...) -> a = b; b = c; ..
Now imagine someone wanting to write a template mixin which given local refs and a premutation, permutes them. If the identity assignment is not allowed, where he will be in a lot of trouble, because he will need to write checks.
In general the identity as a concept should not be hard-special-cased. The compiler is free to optimize it.
Of course, in "simple-enough-cases" the prevention might be worthwhile. But I still think this is a job of another tool - a static code analyzer.

On 1/9/2013 2:50 PM, comco wrote:
> I can forsee someone somewhere having a problem with this if he wants to write a
> general enough templated solution.

You're right that often code that looks wrong written by users, is actually needed by generic code.

On Wednesday, 9 January 2013 at 22:25:53 UTC, Jonathan M Davis wrote:
> On Wednesday, January 09, 2013 23:22:19 Andrej Mitrovic wrote:
>> On 1/9/13, Walter Bright <newshound2@digitalmars.com> wrote:
>> > It is not a bug.
>> 
>> Something related I want to ask you about:
>> 
>> struct S
>> {
>> int _m;
>> this(int m)
>> {
>> this._m = _m; // meant "this._m = m;"
>> }
>> }
>> 
>> I'd like to add a warning for identity assignments when it involves a
>> parameter and a field of the aggregate which is the parent of the
>> function. This was filed as
>> http://d.puremagic.com/issues/show_bug.cgi?id=4407.
>
> Why not just prevent identity assignments altogether? What would be the
> purpose of even allowing a variable to be assigned itself? I see no reason to
> single out parameters or member variables for that.
>
> - Jonathan M Davis


It's very useful as a no-op when debugging.

On 1/10/13, Walter Bright <newshound2@digitalmars.com> wrote:
> You're right that often code that looks wrong written by users, is actually needed by generic code.

Well like I've said, I'm not thinking of warning on identity assignment for all cases, just this particular case. I just need an Ok/Denied so I know whether to spend any time working on the enhancement.

On 1/9/2013 5:05 PM, Andrej Mitrovic wrote:
> On 1/10/13, Walter Bright <newshound2@digitalmars.com> wrote:
>> You're right that often code that looks wrong written by users, is actually
>> needed by generic code.
>
> Well like I've said, I'm not thinking of warning on identity
> assignment for all cases, just this particular case. I just need an
> Ok/Denied so I know whether to spend any time working on the
> enhancement.
>

Denied. I don't see the rationale for in one case but not others.

comco:

> I won't like this code:
> class Pu {
>    int a, b;
>    this(int a, int b) {
>        this.a = a;
>        this.b = b;
>    }
> }
>
> to issue warnings for a and b.

That's a nice warning to have, because that's bug-prone code, worth avoiding.

Bye,
bearophile

H. S. Teoh:

> I used to write code like that too, but then I ran into some nasty
> ambiguity bugs, and now I'm in favor of prohibiting local variables
> shadowing class members. It's just *too* easy to make a mistake, and
> have the code write something to a local variable (which is lost upon
> scope exit) instead of a class member, or vice versa.

Most wise programmers use different argument names like a_,b_ and so on, because it's very easy to create bugs in that situation.

Bye,
bearophile

Jonathan M Davis:

> Why not just prevent identity assignments altogether? What would be the
> purpose of even allowing a variable to be assigned itself?

Despite being sometimes useful in generic code (where?), self-assignment is a common source for bugs.

Bye,
bearophile

On Thursday, January 10, 2013 03:56:10 bearophile wrote:
> comco:
> > I won't like this code:
> > class Pu {
> > 
> > int a, b;
> > this(int a, int b) {
> > 
> > this.a = a;
> > this.b = b;
> > 
> > }
> > 
> > }
> > 
> > to issue warnings for a and b.
> 
> That's a nice warning to have, because that's bug-prone code, worth avoiding.

So, basically, you want the compiler to yell at you for picking a bad variable name? There's _nothing_ to warn about in the code above. It's perfectly valid. I could see warning if you did

this.a = this.a;

or

a = a;

but

this.a = a;

is perfectly valid. And it would be _really_ annoying to not be able to give constructor parameters the same names as the member variables that they're used to set. Their names will typically differ be a _ if the member variables are private, but for POD types, they're almost certainly going to be identical, and warning about that would just be stupide.

- Jonathan M Davis

Walter Bright:

> Denied. I don't see the rationale for in one case but not others.

There is a rationale. In my opinion special cases are bad in a language, unless:
- They always produce a compilation failure in a well defined group of cases. (while just giving a different answer in a vaguely defined group of cases is usually not acceptable);
- They help the programmer avoid a common bug-prone situation.

In D we have already some cases of such special rules, that break uniformity and symmetry to help D programmers write less buggy code.

Issue 4407 (
http://d.puremagic.com/issues/show_bug.cgi?id=4407 ) is/was one of my fifteen most important enhancement requests (and it's a small one). Many of those fifteen are little breaking changes.

I think the case discussed in Issue 4407 passes both conditions:

The cause is defined for class/structs. I think this is a definite enough situation

Regarding commonality:
- I know two tools that detect this bug, the tool that has already found some (different) bugs in the DMD source code and the Google Web Toolkit.
- I have created several bugs because of that, and I am forced to use a coding convention (like adding a trailing _ to argument names in constructors) to avoid them.
- I know three D programmers that have had the same problem of mine, Byron Heads, the OP and another person.
- The static analysis tool I was referring before has uncovered cases of such bug in well used C++ code.

Bye,
bearophile