www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Module level variable shadowing

reply "bearophile" <bearophileHUGS lycos.com> writes:
While I try to minimize the number of module-level variables in D 
code, sometimes they are present, and sometimes I have problems 
(bugs) caused by unwanted shadowing of global names (or sometimes 
more generally, names from outer scopes, but this is less common):


int x = 1;
// ...
void foo() {
     // ...
     int x = 2;
     // ...
     writeln(x);
}


Can't we find a way to avoid/find such kind of bugs? (It's not a 
problem of mutability, because the same problem happens with 
immutable variables.) I have hit this bug several times.

The simplest way to avoid that kind of bugs is give a "shadowing 
global x error" (similar to the shadowing errors D gives with 
foreach and with statements). But this breaks most existing D 
code.

Bye,
bearophile
Jun 25 2014
next sibling parent "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Wed, Jun 25, 2014 at 11:03:14PM +0000, bearophile via Digitalmars-d wrote:
 While I try to minimize the number of module-level variables in D
 code, sometimes they are present, and sometimes I have problems (bugs)
 caused by unwanted shadowing of global names (or sometimes more
 generally, names from outer scopes, but this is less common):
 
 
 int x = 1;
 // ...
 void foo() {
     // ...
     int x = 2;
     // ...
     writeln(x);
 }
 
 
 Can't we find a way to avoid/find such kind of bugs? (It's not a
 problem of mutability, because the same problem happens with immutable
 variables.) I have hit this bug several times.

My advice is to avoid module globals unless absolute necessary. A whole class of problems are avoided that way.
 The simplest way to avoid that kind of bugs is give a "shadowing
 global x error" (similar to the shadowing errors D gives with foreach
 and with statements). But this breaks most existing D code.

If you want to refer to a module global, write .x instead of x. T -- Тише едешь, дальше будешь.
Jun 25 2014
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 6/25/2014 4:03 PM, bearophile wrote:
 The simplest way to avoid that kind of bugs is give a "shadowing global x
error"
 (similar to the shadowing errors D gives with foreach and with statements). But
 this breaks most existing D code.

D has scoped lookup. Taking your proposal as principle, where do we stop at issuing errors when there is the same identifier in multiple in-scope scopes? I think we hit the sweet spot at restricting shadowing detection to local scopes. I suggest that your issues with global variables can be mitigated by adopting a distinct naming convention for your globals. Frankly, I think a global variable named "x" is execrable style - such short names should be reserved for locals.
Jun 25 2014
next sibling parent Jacob Carlborg <doob me.com> writes:
On 2014-06-26 02:41, Walter Bright wrote:

 I suggest that your issues with global variables can be mitigated by
 adopting a distinct naming convention for your globals. Frankly, I think
 a global variable named "x" is execrable style - such short names should
 be reserved for locals.

No need to have a naming convention. As Teoh said, just always prefix the global variables with a dot. -- /Jacob Carlborg
Jun 27 2014
prev sibling parent reply dennis luehring <dl.soluz gmx.net> writes:
Am 26.06.2014 02:41, schrieb Walter Bright:
 On 6/25/2014 4:03 PM, bearophile wrote:
 The simplest way to avoid that kind of bugs is give a "shadowing global x
error"
 (similar to the shadowing errors D gives with foreach and with statements). But
 this breaks most existing D code.

D has scoped lookup. Taking your proposal as principle, where do we stop at issuing errors when there is the same identifier in multiple in-scope scopes? I think we hit the sweet spot at restricting shadowing detection to local scopes. I suggest that your issues with global variables can be mitigated by adopting a distinct naming convention for your globals. Frankly, I think a global variable named "x" is execrable style - such short names should be reserved for locals.

what about adding tests -no-global-shadowing (or others) to dmd and tell people to use it - poeple will definitly change there global names then (like your advised of renameing or using .x etc) and after a while it could become a warning, then an error - like the time between deprecation and removal of an feature - D need more strategies then C++ to add better qualitity over time
Jun 27 2014
parent reply dennis luehring <dl.soluz gmx.net> writes:
Am 27.06.2014 10:20, schrieb dennis luehring:
 I
think we hit the sweet spot at restricting shadowing detection to local scopes.


sweet does not mean - use a better name or .x to avoid manualy hard to detect problems - its like disabled shadow detection in local scopes what i don't understand - why on earth should someone want to shadow a(or better any) variable at all?
Jun 27 2014
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 6/27/2014 1:38 PM, Tofu Ninja wrote:
 On Friday, 27 June 2014 at 08:24:16 UTC, dennis luehring wrote:
 what i don't understand - why on earth should someone want to shadow a(or
 better any) variable at all?

It can be useful if you are using mixins where you don't know what is going to be in the destination scope.

Is true. People who do metaprogramming with C macros have all kinds of problems with the lack of scoping of temp variable names within the macros.
Jun 27 2014
prev sibling next sibling parent reply dennis luehring <dl.soluz gmx.net> writes:
Am 27.06.2014 20:09, schrieb Kapps:
 On Friday, 27 June 2014 at 08:24:16 UTC, dennis luehring wrote:
 Am 27.06.2014 10:20, schrieb dennis luehring:
 I
think we hit the sweet spot at restricting shadowing detection
to local scopes.


sweet does not mean - use a better name or .x to avoid manualy hard to detect problems - its like disabled shadow detection in local scopes what i don't understand - why on earth should someone want to shadow a(or better any) variable at all?

struct Foo { int a; this(int a) { this.a = a; } }

forgot that case - but i don't like how its currently handled, maybe no better way - its just not perfect :)
Jun 27 2014
next sibling parent reply dennis luehring <dl.soluz gmx.net> writes:
Am 28.06.2014 07:11, schrieb H. S. Teoh via Digitalmars-d:
 On Sat, Jun 28, 2014 at 06:37:08AM +0200, dennis luehring via Digitalmars-d
wrote:
 Am 27.06.2014 20:09, schrieb Kapps:

struct Foo {
       int a;
       this(int a) {
           this.a = a;
       }
}

forgot that case - but i don't like how its currently handled, maybe no better way - its just not perfect :)

Actually, this particular use case is very bad. It's just inviting typos, for example, if you mistyped "int a" as "int s", then you get: struct Foo { int a; this(int s) { this.a = a; // oops, now it means this.a = this.a } } I used to like this shadowing trick, until one day I got bit by this typo. From then on, I acquired a distaste for this kind of shadowing. Not to mention, typos are only the beginning of troubles. If you copy a few lines from the ctor into another method (e.g., to partially reset the object state), then you end up with a similar unexpected rebinding to this.a, etc.. Similar problems exist in nested functions: auto myFunc(A...)(A args) { int x; int helperFunc(B...)(B args) { int x = 1; return x + args.length; } } Accidentally mistype "B args" or "int x=1", and again you get a silent bug. This kind of shadowing is just a minefield of silent bugs waiting to happen. No thanks! T

thx for the examples - never though of these problems i personaly would just forbid any shadowing and single-self-assign and then having unique names (i use m_ for members and p_ for parameters etc.) or give a compile error asking for this.x or .x (maybe problematic with inner structs/functions) but that could be a problem for C/C++ code porting - but is that such a big problem?
Jun 27 2014
parent reply Jacob Carlborg <doob me.com> writes:
On 2014-06-28 08:19, dennis luehring wrote:

 thx for the examples - never though of these problems

 i personaly would just forbid any shadowing and single-self-assign
 and then having unique names (i use m_ for members and p_ for parameters
 etc.) or give a compile error asking for this.x or .x (maybe problematic
 with inner structs/functions)

I think, in general, if you need to prefix/suffix any symbols name, there's something wrong with the language. -- /Jacob Carlborg
Jun 28 2014
next sibling parent dennis luehring <dl.soluz gmx.net> writes:
Am 28.06.2014 11:30, schrieb Jacob Carlborg:
 On 2014-06-28 08:19, dennis luehring wrote:

 thx for the examples - never though of these problems

 i personaly would just forbid any shadowing and single-self-assign
 and then having unique names (i use m_ for members and p_ for parameters
 etc.) or give a compile error asking for this.x or .x (maybe problematic
 with inner structs/functions)

I think, in general, if you need to prefix/suffix any symbols name, there's something wrong with the language.

i agree 100% - i just try to overcome the shadowing clean with this AND have also scope information in the name (i just want to know at every palce in code if someing is an parameter) but i would always prefer a better working method
Jun 28 2014
prev sibling parent reply Ary Borenszweig <ary esperanto.org.ar> writes:
On 6/28/14, 6:30 AM, Jacob Carlborg wrote:
 On 2014-06-28 08:19, dennis luehring wrote:

 thx for the examples - never though of these problems

 i personaly would just forbid any shadowing and single-self-assign
 and then having unique names (i use m_ for members and p_ for parameters
 etc.) or give a compile error asking for this.x or .x (maybe problematic
 with inner structs/functions)

I think, in general, if you need to prefix/suffix any symbols name, there's something wrong with the language.

In Ruby the usage of a variable is always prefixed: ` foo` for instance vars, `$foo` for global variable, `FOO` for constant. You can't make a mistake. It's... perfect :-)
Jun 28 2014
next sibling parent dennis luehring <dl.soluz gmx.net> writes:
Am 28.06.2014 14:20, schrieb Ary Borenszweig:
 On 6/28/14, 6:30 AM, Jacob Carlborg wrote:
 On 2014-06-28 08:19, dennis luehring wrote:

 thx for the examples - never though of these problems

 i personaly would just forbid any shadowing and single-self-assign
 and then having unique names (i use m_ for members and p_ for parameters
 etc.) or give a compile error asking for this.x or .x (maybe problematic
 with inner structs/functions)

I think, in general, if you need to prefix/suffix any symbols name, there's something wrong with the language.

In Ruby the usage of a variable is always prefixed: ` foo` for instance vars, `$foo` for global variable, `FOO` for constant. You can't make a mistake. It's... perfect :-)

i like the ruby-way
Jun 28 2014
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2014-06-28 14:20, Ary Borenszweig wrote:

 In Ruby the usage of a variable is always prefixed: ` foo` for instance
 vars, `$foo` for global variable, `FOO` for constant. You can't make a
 mistake. It's... perfect :-)

Oh, that's where you're wrong, very wrong :). Take this for example: class Foo attr_accessor :bar def initialize bar = 3 end def foo puts bar # prints "bar", the instance variable, via a getter bar = 4 puts bar # prints "bar", the local variable puts self.bar # prints "bar", the instance variable, via a getter end end Foo.new.foo -- /Jacob Carlborg
Jun 29 2014
parent Ary Borenszweig <ary esperanto.org.ar> writes:
On 6/29/14, 3:24 PM, Jacob Carlborg wrote:
 On 2014-06-28 14:20, Ary Borenszweig wrote:

 In Ruby the usage of a variable is always prefixed: ` foo` for instance
 vars, `$foo` for global variable, `FOO` for constant. You can't make a
 mistake. It's... perfect :-)

Oh, that's where you're wrong, very wrong :). Take this for example: class Foo attr_accessor :bar def initialize bar = 3 end def foo puts bar # prints "bar", the instance variable, via a getter bar = 4 puts bar # prints "bar", the local variable puts self.bar # prints "bar", the instance variable, via a getter end end Foo.new.foo

That's the only "confusing" thing about Ruby. But I never had troubles with it. In fact, when I use a local variable I never want to call a method with that same name. And when I do, I put a parenthesis. It's a pretty simple rule to learn. A simple solution would be to force parenthesis on method calls that don't have a receiver. So "bar" would always be a variable, "bar()" would be a method call and "foo.bar" and "foo.bar()" would also be method calls.
Jun 29 2014
prev sibling parent dennis luehring <dl.soluz gmx.net> writes:
Am 29.06.2014 08:06, schrieb Kapps:
 struct Foo {
       int a;
       this(this.a) { }
 }

a parameter declaration with the name of the scope name??? totaly different to everything else???
Jun 29 2014
prev sibling parent dennis luehring <dl.soluz gmx.net> writes:
Am 27.06.2014 22:38, schrieb Tofu Ninja:
 On Friday, 27 June 2014 at 08:24:16 UTC, dennis luehring wrote:
 what i don't understand - why on earth should someone want to
 shadow a(or better any) variable at all?

It can be useful if you are using mixins where you don't know what is going to be in the destination scope.

can be usefull in a even more hard to understand situation makes it no better
Jun 27 2014
prev sibling next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Walter Bright:

 I suggest that your issues with global variables can be 
 mitigated by adopting a distinct naming convention for your 
 globals. Frankly, I think a global variable named "x" is 
 execrable style - such short names should be reserved for 
 locals.

I don't use names like 'x' for the global variables, that was just an artificial example. But I am not yet using a naming convention for global variables (like using a "_g" suffix) and perhaps I should start using it. But having something enforced by the compiler is better.
 D has scoped lookup. Taking your proposal as principle, where 
 do we stop at issuing errors when there is the same identifier 
 in multiple in-scope scopes? I think we hit the sweet spot at 
 restricting shadowing detection to local scopes.

From the bugs I've had by unwanted shadowing global variables, the current spot of D doesn't look very sweet to me. I'd like D to try some alternative point where to stop issuing those errors. I am not convinced the current design is the best one. Some kind of error for module-level shadowing could be an improvement over the current situation. I can be wrong of course, but I think it's a good idea to explore some more this little piece of the design space. Bye, bearophile
Jun 26 2014
prev sibling next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
 But having something enforced by the compiler is better.

I meant, it could be better. Bye, bearophile
Jun 26 2014
prev sibling next sibling parent "Kapps" <opantm2+spam gmail.com> writes:
On Friday, 27 June 2014 at 08:24:16 UTC, dennis luehring wrote:
 Am 27.06.2014 10:20, schrieb dennis luehring:
 I
think we hit the sweet spot at restricting shadowing detection 
to local scopes.


sweet does not mean - use a better name or .x to avoid manualy hard to detect problems - its like disabled shadow detection in local scopes what i don't understand - why on earth should someone want to shadow a(or better any) variable at all?

struct Foo { int a; this(int a) { this.a = a; } }
Jun 27 2014
prev sibling next sibling parent "Tofu Ninja" <emmons0 purdue.edu> writes:
On Friday, 27 June 2014 at 08:24:16 UTC, dennis luehring wrote:
 what i don't understand - why on earth should someone want to 
 shadow a(or better any) variable at all?

It can be useful if you are using mixins where you don't know what is going to be in the destination scope.
Jun 27 2014
prev sibling next sibling parent "Meta" <jared771 gmail.com> writes:
On Friday, 27 June 2014 at 20:40:24 UTC, Walter Bright wrote:
 On 6/27/2014 1:38 PM, Tofu Ninja wrote:
 On Friday, 27 June 2014 at 08:24:16 UTC, dennis luehring wrote:
 what i don't understand - why on earth should someone want to 
 shadow a(or
 better any) variable at all?

It can be useful if you are using mixins where you don't know what is going to be in the destination scope.

Is true. People who do metaprogramming with C macros have all kinds of problems with the lack of scoping of temp variable names within the macros.

But keep in mind that we can also name mixins. Or is that only possible when you're mixing in a template/mixin template?
Jun 27 2014
prev sibling next sibling parent "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Sat, Jun 28, 2014 at 06:37:08AM +0200, dennis luehring via Digitalmars-d
wrote:
 Am 27.06.2014 20:09, schrieb Kapps:

struct Foo {
       int a;
       this(int a) {
           this.a = a;
       }
}

forgot that case - but i don't like how its currently handled, maybe no better way - its just not perfect :)

Actually, this particular use case is very bad. It's just inviting typos, for example, if you mistyped "int a" as "int s", then you get: struct Foo { int a; this(int s) { this.a = a; // oops, now it means this.a = this.a } } I used to like this shadowing trick, until one day I got bit by this typo. From then on, I acquired a distaste for this kind of shadowing. Not to mention, typos are only the beginning of troubles. If you copy a few lines from the ctor into another method (e.g., to partially reset the object state), then you end up with a similar unexpected rebinding to this.a, etc.. Similar problems exist in nested functions: auto myFunc(A...)(A args) { int x; int helperFunc(B...)(B args) { int x = 1; return x + args.length; } } Accidentally mistype "B args" or "int x=1", and again you get a silent bug. This kind of shadowing is just a minefield of silent bugs waiting to happen. No thanks! T -- Designer clothes: how to cover less by paying more.
Jun 27 2014
prev sibling next sibling parent "Kapps" <opantm2+spam gmail.com> writes:
On Saturday, 28 June 2014 at 05:13:16 UTC, H. S. Teoh via 
Digitalmars-d wrote:
 Actually, this particular use case is very bad. It's just 
 inviting
 typos, for example, if you mistyped "int a" as "int s", then 
 you get:

 	struct Foo {
 		int a;
 		this(int s) {
 			this.a = a; // oops, now it means this.a = this.a
 		}
 	}

While I'd much prefer a syntax to handle this automatically, like: struct Foo { int a; this(this.a) { } } I prefer having the variable names match what they're assigning since it makes it clearer. Some other benefits include being able to get documentation for free. When I was making my own IDE plugin a year ago, one of the features I liked was that if a parameter in the constructor had the same name as a field/property in the class, documentation would be shown for said field/property as well as for the parameter. It's just a matter of personal preference. In theory your compiler should tell you if you're making a useless assignment like the one in the example.
Jun 28 2014
prev sibling next sibling parent "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Saturday, 28 June 2014 at 12:20:15 UTC, Ary Borenszweig wrote:
 On 6/28/14, 6:30 AM, Jacob Carlborg wrote:
 On 2014-06-28 08:19, dennis luehring wrote:

 thx for the examples - never though of these problems

 i personaly would just forbid any shadowing and 
 single-self-assign
 and then having unique names (i use m_ for members and p_ for 
 parameters
 etc.) or give a compile error asking for this.x or .x (maybe 
 problematic
 with inner structs/functions)

I think, in general, if you need to prefix/suffix any symbols name, there's something wrong with the language.

In Ruby the usage of a variable is always prefixed: ` foo` for instance vars, `$foo` for global variable, `FOO` for constant. You can't make a mistake. It's... perfect :-)

OTOH, the distinction between methods and local variables isn't as easy to see. It's still deterministic, albeit using non-obvious syntactic rules.
Jun 29 2014
prev sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
H. S. Teoh:

 I used to like this shadowing trick, until one day I got bit by 
 this typo. From then on, I acquired a distaste for this kind of 
 shadowing.

See: https://d.puremagic.com/issues/show_bug.cgi?id=3878 Bye, bearophile
Jun 29 2014