On Thu, Oct 23, 2014 at 10:33:53AM +0530, Shriramana Sharma via Digitalmars-d-learn wrote:
> Hello. Please see the following code:
> 
> import std.stdio ;
> struct Pair {
> 	int x, y ;
> 	this (int x, int y) { x = x ; y = y ; }

Please don't write code like this.  How is the compiler supposed to know that "x = x" is supposed to mean "this.x = x", as opposed to "x = x" or "this.x = this.x" or "x = this.x"??!

I used to enjoy writing this kind of code by cleverly exploiting subtle differences in symbol lookup rules, but eventually I realized that this kind of coding style is riddled with subtle errors, because shadowing member variables can lead to nasty mistakes (writing 'x' in your member functions can sometimes mean this.x, sometimes a function parameter or local variable) and makes the code unmaintainable (anybody else reading the code may not be aware of the subtle lookup rules you're exploiting to make the code work) and fragile (code changes like adding/removing a local variable may inadvertently change the meaning of 'x' in another line of code).

Generally, shadowing member variables with parameter names is a Bad Idea(tm). Instead, my advice is to adopt a naming convention to distinguish between them, e.g.:

	struct S {
		int x, y;
		this(int _x, int _y) {
			x = _x; // now no longer ambiguous
			y = _y;
		}
	}


T

-- 
Don't modify spaghetti code unless you can eat the consequences.

Hello. I perfectly realize that it's not advisable to take advantage of shadowing. In fact, I asked the question because I thought D specifically *didn't* allow shadowing, but here it is, being silently permitted to mishappen... I seem to read to have read that (D didn't allow shadowing) but I'm not able to point to where...

BTW I compiled it with LDC 0.14.0 and separately with D 2.066.0 and both give the same result.

So shouldn't the compiler ideally complain about the arguments shadowing members?


-- 
Shriramana Sharma ஶ்ரீரமணஶர்மா श्रीरमणशर्मा

On Thu, Oct 23, 2014 at 11:01:10AM +0530, Shriramana Sharma via Digitalmars-d-learn wrote:
> Hello. I perfectly realize that it's not advisable to take advantage of shadowing. In fact, I asked the question because I thought D specifically *didn't* allow shadowing, but here it is, being silently permitted to mishappen... I seem to read to have read that (D didn't allow shadowing) but I'm not able to point to where...
> 
> BTW I compiled it with LDC 0.14.0 and separately with D 2.066.0 and both give the same result.
> 
> So shouldn't the compiler ideally complain about the arguments shadowing members?
[...]

File a bug, if one isn't already filed. (I vaguely seem to recall an existing bug to that effect, but I could be wrong.)


T

-- 
What do you call optometrist jokes? Vitreous humor.

Oh OK here it is: http://dlang.org/deprecate.html#Variable%20shadowing%20inside%20functions

But it says "it is an error to use the feature" by 2.061 and I'm using 2.066!

Doesn't the scope of that deprecation cover struct members too? In general it should be like "variables in inner scopes should not shadow those in outer scopes" irrespective of what the scopes are, right?

Of course, for clarity we could add "except when the inner scope is a separate namespace", so it is made clear that enum/struct/class members do not shadow outer variables...


-- 
Shriramana Sharma ஶ்ரீரமணஶர்மா श्रीरमणशर्मा

On Thursday, October 23, 2014 11:47:04 Shriramana Sharma via Digitalmars-d-learn wrote:
> Oh OK here it is:
> http://dlang.org/deprecate.html#Variable%20shadowing%20inside%20functions
>
> But it says "it is an error to use the feature" by 2.061 and I'm using
> 2.066!
>
> Doesn't the scope of that deprecation cover struct members too? In
> general it should be like "variables in inner scopes should not shadow
> those in outer scopes" irrespective of what the scopes are, right?
>
> Of course, for clarity we could add "except when the inner scope is a
> separate namespace", so it is made clear that enum/struct/class
> members do not shadow outer variables...

Shadowing is only illegal when you're dealing with local variables both declared within the same function with one in an inner scope. It's perfectly legal when one variable is a local variable and the other is a member variable, class/struct variable, or module-level variable. Having a parameter shadow a member variable is perfectly okay, because you can still reference the member variable via the invisible this parameter.

Questions like this have come up and been discussed before, but using the same parameter names as member variable names for constructors is such a common practice that there would be quite a bit of screaming if we didn't allow it. It's not going away, and personally, I'd be very annoyed if it did. It's wonderfully clear when the parameter name has the same name as the member variable that it's going to initialize, and I'd hate to have to come up with different parameter names just because someone was worried about someone being foolish enough to do

x = x;

which is so obviously wrong that I don't know how much of anyone could make that mistake. But simply making it illegal to assign a variable to itself would solve that problem, and that arguably should be done, since it's a essentially a no-op.

Regardless, a lot of people solve it by simply tagging their member variables in some manner to indicate that they're member variables and not local variables - e.g. _x instead of x.

- Jonathan M Davis

Jonathan M Davis:

> Questions like this have come up and been discussed before, but using the same parameter names as member variable names for constructors is such a common practice that there would be quite a bit of screaming if we didn't allow it.

I'm willing to hear them scream. D should statically forbid such kind of code. I have had many (usually quick to find) bugs caused by this.

Bye,
bearophile

On 10/22/2014 11:37 PM, Jonathan M Davis wrote:

> x = x;
>
> which is so obviously wrong that I don't know how much of anyone could
> make that mistake. But simply making it illegal to assign a variable to
> itself would solve that problem, and that arguably should be done, since
> it's a essentially a no-op.

Steve said the same thing and it's true for fundamental types but just to annoy you two, assignment can have side effects. :)

import std.stdio;

struct X
{
    X opAssign(X that)
    {
        writeln("side-effect!");
        return this;
    }
}

void main()
{
    auto x = X();
    x = x;
}

Ali

On Thursday, 23 October 2014 at 18:57:33 UTC, Ali Çehreli wrote:
> On 10/22/2014 11:37 PM, Jonathan M Davis wrote:
>
> > x = x;
> >
> > which is so obviously wrong that I don't know how much of
> anyone could
> > make that mistake. But simply making it illegal to assign a
> variable to
> > itself would solve that problem, and that arguably should be
> done, since
> > it's a essentially a no-op.
>
> Steve said the same thing and it's true for fundamental types but just to annoy you two, assignment can have side effects. :)

But it wouldn't be a no-op then.

On Thursday, 23 October 2014 at 18:57:33 UTC, Ali Çehreli wrote:
> On 10/22/2014 11:37 PM, Jonathan M Davis wrote:
>
> > x = x;
> >
> > which is so obviously wrong that I don't know how much of
> anyone could
> > make that mistake. But simply making it illegal to assign a
> variable to
> > itself would solve that problem, and that arguably should be
> done, since
> > it's a essentially a no-op.
>
> Steve said the same thing and it's true for fundamental types but just to annoy you two, assignment can have side effects. :)
>
> import std.stdio;
>
> struct X
> {
>     X opAssign(X that)
>     {
>         writeln("side-effect!");
>         return this;
>     }
> }
>
> void main()
> {
>     auto x = X();
>     x = x;
> }

LOL. Yeah. In that case, it wouldn't be a no-op and shouldn't be an error, but in any assignment where the operator wasn't overloaded by that type or any of its member variables (directly or indirectly), it _is_ a no-op and should arguably result in an error.

- Jonathan M Davis

On 10/23/14 3:22 PM, "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm@gmx.net>" wrote:
> On Thursday, 23 October 2014 at 18:57:33 UTC, Ali Çehreli wrote:
>> On 10/22/2014 11:37 PM, Jonathan M Davis wrote:
>>
>> > x = x;
>> >
>> > which is so obviously wrong that I don't know how much of
>> anyone could
>> > make that mistake. But simply making it illegal to assign a
>> variable to
>> > itself would solve that problem, and that arguably should be
>> done, since
>> > it's a essentially a no-op.
>>
>> Steve said the same thing and it's true for fundamental types but just
>> to annoy you two, assignment can have side effects. :)
>
> But it wouldn't be a no-op then.

Was about to say the same thing :)

I think the compiler might be able to tell the difference between x = x; when x is an int and x is a struct with opAssign.

-Steve