Discussion:
Constructor params with same name as members
Shriramana Sharma via Digitalmars-d-learn
2014-10-23 05:03:53 UTC
Permalink
Hello. Please see the following code:

import std.stdio ;
struct Pair {
int x, y ;
this (int x, int y) { x = x ; y = y ; }
}
void main() {
auto P = Pair(1, 2) ;
writeln(P.x, ' ', P.y) ;
}

This outputs 0 0, whereas the equivalent C++ code outputs 1 2 correctly:

# include <iostream>
struct Pair {
int x, y ;
Pair(int x, int y) : x(x), y(y) {}
} ;
int main() {
auto P = Pair(1, 2) ;
std::cout << P.x << ' ' << P.y << std::endl ;
}

It seems to me that D should either not permit argument names to
shadow the member names, since it has no initializer lists and all
members are automatically initialized. Comments?
--
Shriramana Sharma ஶ்ரீரமணஶர்மா श्रीरमणशर्मा
H. S. Teoh via Digitalmars-d-learn
2014-10-23 05:18:13 UTC
Permalink
Post by Shriramana Sharma via Digitalmars-d-learn
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.
Shriramana Sharma via Digitalmars-d-learn
2014-10-23 05:31:10 UTC
Permalink
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 ஶ்ரீரமணஶர்மா श्रीरमणशर्मा
H. S. Teoh via Digitalmars-d-learn
2014-10-23 05:33:02 UTC
Permalink
Post by Shriramana Sharma via Digitalmars-d-learn
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.
Shriramana Sharma via Digitalmars-d-learn
2014-10-23 06:17:04 UTC
Permalink
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 ஶ்ரீரமணஶர்மா श्रीरमणशर्मा
Jonathan M Davis via Digitalmars-d-learn
2014-10-23 06:37:00 UTC
Permalink
On Thursday, October 23, 2014 11:47:04 Shriramana Sharma via
Post by Shriramana Sharma via Digitalmars-d-learn
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
bearophile via Digitalmars-d-learn
2014-10-23 08:15:52 UTC
Permalink
Post by Jonathan M Davis via Digitalmars-d-learn
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
Ali Çehreli via Digitalmars-d-learn
2014-10-23 18:57:33 UTC
Permalink
Post by Jonathan M Davis via Digitalmars-d-learn
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
via Digitalmars-d-learn
2014-10-23 19:22:41 UTC
Permalink
Post by Jonathan M Davis via Digitalmars-d-learn
Post by Jonathan M Davis via Digitalmars-d-learn
x = x;
which is so obviously wrong that I don't know how much of
anyone could
Post by Jonathan M Davis via Digitalmars-d-learn
make that mistake. But simply making it illegal to assign a
variable to
Post by Jonathan M Davis via Digitalmars-d-learn
itself would solve that problem, and that arguably should be
done, since
Post by Jonathan M Davis via Digitalmars-d-learn
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.
Jonathan M Davis via Digitalmars-d-learn
2014-10-23 20:56:51 UTC
Permalink
Post by Jonathan M Davis via Digitalmars-d-learn
Post by Jonathan M Davis via Digitalmars-d-learn
x = x;
which is so obviously wrong that I don't know how much of
anyone could
Post by Jonathan M Davis via Digitalmars-d-learn
make that mistake. But simply making it illegal to assign a
variable to
Post by Jonathan M Davis via Digitalmars-d-learn
itself would solve that problem, and that arguably should be
done, since
Post by Jonathan M Davis via Digitalmars-d-learn
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

Steven Schveighoffer via Digitalmars-d-learn
2014-10-23 13:39:43 UTC
Permalink
Post by Shriramana Sharma via Digitalmars-d-learn
import std.stdio ;
struct Pair {
int x, y ;
this (int x, int y) { x = x ; y = y ; }
}
void main() {
auto P = Pair(1, 2) ;
writeln(P.x, ' ', P.y) ;
}
# include <iostream>
struct Pair {
int x, y ;
Pair(int x, int y) : x(x), y(y) {}
} ;
This is not the same. In the above, the x outside the parens is ALWAYS a
member. D does not have this syntax. Change it to the same as your D
implementation, and you get the same result (actually worse, because C++
will not initialize x and y for you).
Post by Shriramana Sharma via Digitalmars-d-learn
int main() {
auto P = Pair(1, 2) ;
std::cout << P.x << ' ' << P.y << std::endl ;
}
It seems to me that D should either not permit argument names to
shadow the member names, since it has no initializer lists and all
members are automatically initialized. Comments?
You're missing the "or" part of that statement :)

But 2 things:

x = x;

This should produce an error, or at least a warning I think, as it does
nothing. However, even with dmd -w, it does not. I know I have seen the
compiler complain about noop statements before, I just don't know under
what circumstances

and 2, you would use the same mechanism as you use with C++ to
initialize the items inside the ctor:

this.x = x;

Note, the rules for shadowing are the same as for any function
parameters to any function vs. members or module variables. Nothing is
inconsistent here.

-Steve
Continue reading on narkive:
Loading...