www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - More name hiding

reply bearophile <bearophileHUGS lycos.com> writes:
After translating some buggy C code (full of gotos and global variables) to D
and going hunting for bugs for some time, I have grown a desire to write this
post.

Variable name hiding is a source of bugs. A typical example:


int x;
void foo() {
    int x = 5;
    // lot of code code here
    x++;
}


The local name 'x' hides the global variable name 'x'. Sometimes the programmer
thinks she is using a global variable, when instead she is using the local one,
of vice versa. Coding guidelines suggest to rename such local variable x to
avoid possible future troubles, and I agree with this advice.

Some ways to face this problem:

1) Do nothing.
2) Give optional explicit information about the source of all variable names
used in a function, using the optional  outer() I have proposed elsewhere.
3) Statically disallow (gives an error) local variables from shadowing global
ones.
4) Produce a warning when a local variable shadows a global ones, in a smart
way.
5) As the leading dot to denote global variables, require the use of another
leading symbol (or some other mean) to denote local variable, when there is
ambiguity.

--------------

Regarding option 1, in D global variables are uncommon if you follow modern
programming practices, and if you keep functions short it's easy to see if they
are shadowing an outer name.
But the situation is sometimes different if you are porting legacy C code to D,
or if you have bad D programmers :-)

--------------

Option 3 is not so bad, D language already contains some examples of this:

struct Foo { int j; }
void main() {
    foreach (i; 0 .. 10)
        foreach (i; 0 .. 10) {} // error
    Foo f;
    int j;
    with (f) {
        j++; // error
    }
}

--------------

Option 4 too is not bad, especially if some care is given to avoid producing
warnings where they are not needed:


int x, y, z;
void main() {
  int x;
  .x++; // no warning, the scope is known
  x++; // warning
  string y;
  y = "foo"; // no warning, .y y have incompatible types
  int z;
  float z;
  z++; // warning, int is implicitly convertible to float
}

--------------

Option 5 means requiring explicit scope information when there there is a
collision:

int x;
void main() {
  int x;
  .x++; // no error, the scope is known
  main.x++; // no error, the scope is known
}

--------------

I have discussed about hiding global variables from local ones, but D allows
nesting of functions, so this discussion applies to such variables too:


void main() {
    int x;
    void foo() {
        int x;
        // code here
        x++;
    }
}

Bye,
bearophile
Aug 19 2011
next sibling parent reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
It's not just globals. It's hiding fields as well:

class A
{
    int x;
    this()
    {
         int x; // <- hides this.x
    }
}

I've had this happen during a cut/paste session, which happens when
I'm refactoring my code.
Aug 19 2011
parent reply Mehrdad <wfunction hotmail.com> writes:
On 8/19/2011 8:17 AM, Andrej Mitrovic wrote:

 It's not just globals. It's hiding fields as well:

 class A
 {
      int x;
      this()
      {
           int x; //<- hides this.x
      }
 }

 I've had this happen during a cut/paste session, which happens when
 I'm refactoring my code.


I actually use this on PURPOSE -- I always use the 'this' pointer when 
referring to members anyway. It's a great way to avoid unnecessarily 
renaming x to things like x1, x_1, x_, _x, x0, x_0, this_x, my_x, myx, 
etc...

Even more common with constructor parameters and instance fields.
Aug 19 2011
parent reply Kagamin <spam here.lot> writes:
Mehrdad Wrote:


 Even more common with constructor parameters and instance fields.


If a naming convention encourages shadowing, it's a broken naming convention.
Aug 20 2011
parent reply Mehrdad <wfunction hotmail.com> writes:
On 8/20/2011 3:46 AM, Kagamin wrote:

 Mehrdad Wrote:

 Even more common with constructor parameters and instance fields.


 If a naming convention encourages shadowing, it's a broken naming convention.


I don't think a convention should /encourage/ shadowing per se, but I 
don't think it should discourage it either, except for local variable 
declarations. Parameters shadowing outside members is pretty common and 
useful, although it really depends on whether the user uses "this" 
habitually or not.
Aug 20 2011
parent reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
How the hell is it useful? If you have:

{
    int x;
    {
        double x;
        // use x
    }
}

And you comment out or remove "double x", you've just introduced a bug
in your code.
Aug 20 2011
parent Mehrdad <wfunction hotmail.com> writes:
 Parameters shadowing outside members is pretty common and useful





 How the hell is it useful?
 If you have:
 {
     int x;
     {
         double x; // use x
     }
 }
 And you comment out or remove "double x", you've just introduced a 


bug in your code

I think you missed the word *MEMBER* in my post. I specifically said 
that I'm talking about MEMBER variables, not local variables.
Aug 20 2011
prev sibling parent reply nsf <no.smile.face gmail.com> writes:
On Fri, 19 Aug 2011 11:07:12 -0400
bearophile <bearophileHUGS lycos.com> wrote:


 Some ways to face this problem:
 
 1) Do nothing.
 2) Give optional explicit information about the source of all variable names
used in a function, using the optional  outer() I have proposed elsewhere.
 3) Statically disallow (gives an error) local variables from shadowing global
ones.
 4) Produce a warning when a local variable shadows a global ones, in a smart
way.
 5) As the leading dot to denote global variables, require the use of another
leading symbol (or some other mean) to denote local variable, when there is
ambiguity.


Well, D currently says I can't shadow variables:

void foo() {
	int x = 10;
	{
		int x = 5; // error
	}
}

which pisses me off personally. It's such a handy feature. And there is
exactly one way to solve your problem which comes in two parts:

1. Don't violate the rule: "one or two screens of text per function".
2. Keep the low number of variables in a function, people say it should
be close to seven.

/me wants to be able to shadow his variables everywhere
Aug 19 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
nsf:


 Well, D currently says I can't shadow variables:


Right, that's a third way to shadow variables that D forbids, thank you. But in
my post I've shown other kinds of shadowing that currently D accepts. Shadowing
global variables has caused some problems in my C code. One good commercial C
lint lists all such cases with a warning.



 which pisses me off personally. It's such a handy feature.


It's a handy anti-feature, good source of bugs. Style guides suggest to rename
one of them. If I see production C/C++ code that does that, I rename one of the
two variables. In Haskell in your situation you are allowed to use x and x'.



 And there is exactly one way to solve your problem which comes in two parts:


Surely there are some alternative ways to solve this problem, SPARK and Java
doesn't use your solution.

 

 1. Don't violate the rule: "one or two screens of text per function".
 2. Keep the low number of variables in a function, people say it should
 be close to seven.


I like to add a third rule, for D code:
3. Use pure functions everywhere possible. This avoids the shadowing problem.

But:
1) Old C code translated to D often doesn't follow your rules, unfortunately.
2) I have had colleagues that don't always follow your rules.
3) Even good codebases sometimes have longer functions. You find them even in
the C sources of Python.
4) Even if you always follow your two rules, you are able to shadow a global
(or outer) variable still, and 20 lines later sometimes you think that you are
using the global variable, while you are modifying the local one. I have seen
this happen even in code written by expert programmers, and it causes problems.



 /me wants to be able to shadow his variables everywhere


Good luck :-)

Bye and thank you for your comments,
bearophile
Aug 19 2011
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 08/20/2011 06:54 AM, bearophile wrote:
nsf:


 Well, D currently says I can't shadow variables:


 Right, that's a third way to shadow variables that D forbids, thank
 you. But in my post I've shown other kinds of shadowing that
 currently D accepts. Shadowing global variables has caused some
 problems in my C code. One good commercial C lint lists all such
 cases with a warning.


In some cases, shadowing can be handy for code generation via string 
mixins (the shadowing that is disallowed, not the shadowing of globals). 
But it has also statically caught what would have been a bug in two or 
three cases. I do not mind if the compiler ensures that there is no 
shadowing of globals, because

1. I typically have almost no globals.
2. I don't name globals in a way that is likely to interfere.


Disallowing shadowing of variables inside nested functions might be 
annoying though, especially if it would apply to parameters as well.



 which pisses me off personally. It's such a handy feature.


 It's a handy anti-feature, good source of bugs. Style guides suggest
 to rename one of them. If I see production C/C++ code that does that,
 I rename one of the two variables. In Haskell in your situation you
 are allowed to use x and x'.


Which would be really nice to have in D too. Afaik all it would require 
would be a simple change to the lexer.
Aug 20 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
Timon Gehr:


 In some cases, shadowing can be handy for code generation via string 
 mixins (the shadowing that is disallowed, not the shadowing of globals).


Making the language more forbidding makes the generation of code simpler. But
it's simpler to write code that generates correct code, than for humans to
write bug-free code. So it's much more important to design the language to be
good (not bug-prone) to be written by hand.

Bye,
bearophile
Aug 20 2011
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 08/20/2011 08:21 PM, bearophile wrote:

 Timon Gehr:


 In some cases, shadowing can be handy for code generation via string
 mixins (the shadowing that is disallowed, not the shadowing of globals).


 Making the language more forbidding makes the generation of code simpler. But
it's simpler to write code that generates correct code, than for humans to
write bug-free code. So it's much more important to design the language to be
good (not bug-prone) to be written by hand.

 Bye,
 bearophile


Actually, since some forms of shadowing are still allowed, it makes the 
generation of code slightly more complex. If your other statement holds 
true, we can as well allow shadowing and require all code to be wrapped 
inside a string mixin instead :o). I consider code generation via string 
mixins something the language is worth optimizing for, because it is so 
useful. Fighting with the compiler about shadowed variables in the 
generated code is not pleasant, because you have to resort to ugly 
workarounds to make it shut up. (those then may introduce bugs in your 
code generating code).
Aug 20 2011
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 08/20/2011 08:37 PM, Timon Gehr wrote:

 On 08/20/2011 08:21 PM, bearophile wrote:

 Timon Gehr:


 In some cases, shadowing can be handy for code generation via string
 mixins (the shadowing that is disallowed, not the shadowing of globals).


 Making the language more forbidding makes the generation of code
 simpler. But it's simpler to write code that generates correct code,
 than for humans to write bug-free code. So it's much more important to
 design the language to be good (not bug-prone) to be written by hand.

 Bye,
 bearophile


 Actually, since some forms of shadowing are still allowed, it makes the
 generation of code slightly more complex.
 [snip.]


Oh, disregard that first sentence.

How does disallowing shadowing make code generation via string mixins 
simpler?
Aug 20 2011