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.

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.


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 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 

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 next 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:

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.

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
prev sibling parent 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