Bug 3878 – Arguments and members with the same name

This D2 program compiles: class Foo { int x; this(int x) { x = x; } void inc(int x) { this.x += x; } } class Bar { int x; this() { x = 5; } } struct Spam { static int x; void inc(int x) { Spam.x += x; } } void main() {} The compiler has no problems in knowing what variables are each of those (and maybe a good IDE too can tell them apart well, and give different colors to arguments and attributes), but for the eye of a programmer that's not using such IDE that code is confusing enough (because the "this." is optional in D). I see two possible solutions to avoid that mess: The solution I prefer: the compiler can give an error when an argument has the same name of an attribute. This is more tidy. Less restricting solution: in only those cases, where there can be ambiguity for the eyes of a programmer, the compiler can require the use of "this.". Discussion: http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=106970

A comment by Clemens (http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=106995 ) about the second idea: >As soon as you have an instance variable x and a function parameter x, there will *always* be ambiguity (unless you don't use the parameter). Requiring the use of "this." amounts to disallowing access to the parameter x. Or do you suggest making the distinction based on whether x is used as an lvalue or a rvalue?<

As I understand option(2) of this proposal, if a function parameter 'shadows' a member variable of the same name, it should be illegal to use that parameter as an lvalue. I think you're probably correct in claiming that any such code is highly likely to be a bug. If x is both a parameter and a member, then x = 3; should almost certainly be this.x = 3;

See related bug 4407

See also "Using Redundancies to Find Errors", by Yichen Xie and Dawson Engler, 2002: www.stanford.edu/~engler/p401-xie.pdf

The summary was confusing - many of us understand "attributes" to mean the keywords that qualify a declaration. It took me few moments to make sense of it. (In reply to comment #0) > Less restricting solution: in only those cases, where there can be ambiguity > for the eyes of a programmer, the compiler can require the use of "this.". It already is required when you want to access the member in such circumstances.

(In reply to comment #5) > (In reply to comment #0) > > Less restricting solution: in only those cases, where there can be ambiguity > > for the eyes of a programmer, the compiler can require the use of "this.". > > It already is required when you want to access the member in such > circumstances. I don't know what cases you refer to. Probably it's required when there is ambiguity for the compiler; but I have said "for the eyes of a programmer". Human eyes are not as good as compilers. A good language as D needs to prevent human mistakes when possible.

(In reply to comment #6) > (In reply to comment #5) > >> (In reply to comment #0) >>> Less restricting solution: in only those cases, where there can >>> be ambiguity for the eyes of a programmer, the compiler can >>> require the use of "this.". >> >> It already is required when you want to access the member in such >> circumstances. > > I don't know what cases you refer to. If a function parameter or local variable has the same name as a member of the class/struct/union in which it is defined, then in order to access the latter within the former's scope one must use "this.". Isn't that obvious? > Probably it's required when there is > ambiguity for the compiler; but I have said "for the eyes of a programmer". So you think that "this." should be required for all accesses to members even when there's no ambiguity? This could make code rather cluttered. > Human eyes are not as good as compilers. A good language as D needs to prevent > human mistakes when possible. You mean like accidentally naming a local variable the same as a member of the class, except that at the same time you forget to declare it? The real solution to the original issue would be to (a) invent an explicit notation to access a local variable or function parameter (b) make it mandatory if it's ambiguous with anything else. But what would the scope of "anything else" be? * Just members of the class? * Members of the class and any of its outer classes? * Anything declared in the same module that is accessible from within the function? * Anything declared anywhere in the import tree that is accessible from within the function?

Here we could benefit from const parameters by default. Though this won't help in a case when local variable shadows member.

In this enhancement request I am asking for ideas to avoid a common but specific kind of bug. One idea (not mine) to avoid some of those bugs is to introduce the this.fieldname syntax for method argument (type is optional and it may be different from the field type). In the constructor the argument assigns directly to the field: class Foo { int x; this(this.x) {} void inc(int x) { this.x += x; } }

(In reply to comment #9) A simpler solution are classes with automatic constructors: class Foo { string x; int y = 1; } void main() { Foo f1 = new Foo(); // Good Foo f2 = new Foo("hello"); // Good Foo f3 = new Foo("hello", 10); // Good } Some comments from a discussion thread: ----------------- Daniel Gibson: >> A simpler solution are classes with automatic constructors: >> >> class Foo { >> string x; >> int y = 1; >> } >> void main() { >> Foo f1 = new Foo(); // Good >> Foo f2 = new Foo("hello"); // Good >> Foo f3 = new Foo("hello", 10); // Good >> } >> >> >> What kind of problems are caused by this? :-) > > You'd have to know the order in that the members are defined in the > class (and you may not change the order). > Just imagine > class Foo { > int bla; > int baz; > } > > new Foo(42, 3); // what is bla, what is baz? > > and then you decide "uh I'd prefer to have my class members ordered > alphabetically" and *bamm* all you code silently breaks. > > having a this(this.bla, this.baz) {} would clearly document which > argument in the constructor belongs to which class member and the class > members ordering wouldn't matter. ----------------- Don: > I agree. But unfortunately, the idea is a relatively complicated feature > with a lot of special cases. For example, this(this.bla, this.bla){} > and what if the class contains a union and you set multiple members of it? > The whole thing is actually quite messy. It's not _terrible_, but it's > far from trivial, and it's more complicated than some far more powerful > and useful language features. ----------------- KennyTM~: >> I agree. But unfortunately, the idea is a relatively complicated feature >> with a lot of special cases. For example, this(this.bla, this.bla){} > > 'int f(int x, int x) {}' is a syntax error. So should 'this(this.x, > this.x){}'. ----------------- Daniel Gibson: >> 'int f(int x, int x) {}' is a syntax error. So should 'this(this.x, this.x){}'. > > and probably this(this.x, x){} ----------------- Don: > It's not that it's complicated, it's that for structs, the ordering of > their members is part of the public interface. For classes, the members > are not public. >> and probably this(this.x, x){} > > Exactly. That's why it's messier than it first appears. > My point is -- people tend to think things like this are trivial > features because they are not very powerful; and conversely, they think > that powerful features must be complicated. But that's really > misleading. 'pure', for example, is roughly the same level of > implementation complexity as this feature. ----------------- KennyTM~: >> and probably this(this.x, x){} > > Yes. This is handled by the AST transform (lowering) too. > > this(this.x, int x) { > statements; > } > > becomes > > this(typeof(this.x) x, int x) { > this.x = x; > statements; > } > > which will complain > > Error: constructor x.Foo.this parameter this.x is already defined > > as expected. ----------------- Overall I think the syntax this(this.x){} causes less troubles (class field keep being private, their order doesn't need to become public), while avoiding the bug that's the main topic of this enhancement request.

This enhancement request packs two different things: a request for an error message, and an alternative request for syntax sugar. To manage the two ideas it's better to split them in two. So this ER is just about the error message. The other half is in the new Issue 9801.

In reply to the original report, I disagree that this is a significant issue, in fact I'll often deliberately name a parameter the same as a member. struct S { int m; this(int m) { this.m = m; } } because I like to make it clear I am initializing m. I find code like this to be unduly wretched: struct S { int m; this(int _m) { m = _m; } } I'm opposed to this enhancement.

(In reply to comment #12) > In reply to the original report, I disagree that this is a significant issue, > in fact I'll often deliberately name a parameter the same as a member. > > struct S { > int m; > this(int m) { this.m = m; } > } Thank you for the answer. From my experience that code is very bug prone, I have found various bugs in code like that. I am not the only D programmer to hit such problem. Two alternative solutions: 1) To allow to refer to the instance fields in the constructor signature, so you don't need to repeat names (something like this is present in Scala and Typescript): class S { int m; this(in this.m) {} } 2) Require the usage of "this." in constructors, so you have to write this: struct S { int m; this(int m) { this.m = m; } } The problem is not just about constructors, but they are typically the ones where you initialise most instance members using arguments, so it's where I had my bugs.

If it were to ever happen it needs a DIP. // is super common idiom and nobody is buffled by what is happening here this.x = x; // Now this should be caught be compiler just using the heuristic // "pure expression with its results discarded" x = x;

I am also opposed to this enhancement. This check may be implemented by a third party tool using dmd as a library. In fact, IIRC d-scanner already has such a check. Since it is a matter of preference, folks can use it or not by running D-scanner on their code. Closing as WONTFIX.