Shouldn't this code give a shadowing error?

int x;                          // global?

void test() @safe nothrow @nogc
{
    int x = x;                  // shouldn't this give a shadowing warning?
}

On Friday, 14 August 2020 at 08:58:04 UTC, Per Nordlöw wrote:
> Shouldn't this code give a shadowing error?
>
> int x;                          // global?
>
> void test() @safe nothrow @nogc
> {
>     int x = x;                  // shouldn't this give a shadowing warning?
> }

Try to deprecate it and see how much breaks ;)
The real offender is not this though, it's shadowing members from super classes.

On Friday, 14 August 2020 at 08:58:04 UTC, Per Nordlöw wrote:
> Shouldn't this code give a shadowing error?
>
> int x;                          // global?
>
> void test() @safe nothrow @nogc
> {
>     int x = x;                  // shouldn't this give a shadowing warning?
> }

Why should it ? they are not declared in the same scope so resoltuion rules work perfectly, you distinguish both in several ways. The global using `modulename.x` or `.x` and just `x` in the func body to use the local one.

On Friday, 14 August 2020 at 09:41:27 UTC, Mathias LANG wrote:
> The real offender is not this though, it's shadowing members from super classes.

Ok, thanks.

On 8/14/20 4:58 AM, Per Nordlöw wrote:
> Shouldn't this code give a shadowing error?
> 
> int x;                          // global?
> 
> void test() @safe nothrow @nogc
> {
>      int x = x;                  // shouldn't this give a shadowing warning?
> }
> 

I don't know if this is a hard-fast rule, but my view of the shadowing error is that if you can access the variable, it's not shadowing.

You can access the global via .x

But in the following, there is no way to access the outer x:

void test()
{
   int x; // x1
   foreach(x; [1, 2, 3]) { /* how to access x1 here? */ }
}

> The real offender is not this though, it's shadowing members from super classes. 

Super members can also be accessed via scope naming: super.x

-Steve

On 8/14/2020 5:51 AM, Steven Schveighoffer wrote:
> On 8/14/20 4:58 AM, Per Nordlöw wrote:
>> Shouldn't this code give a shadowing error?
>>
>> int x;                          // global?
>>
>> void test() @safe nothrow @nogc
>> {
>>      int x = x;                  // shouldn't this give a shadowing warning?
>> }
>>
> 
> I don't know if this is a hard-fast rule, but my view of the shadowing error is that if you can access the variable, it's not shadowing.
> You can access the global via .x

It's a reasonable way to put it.

More to Per's query, the current behavior is not an accident nor a bug, it is deliberate.

It came about because in my long experience coding, I had repeated bugs due to shadowing of locals. It was difficult to tell, for longer functions, if a variable had already been declared. Even when done correctly, shadowing declarations made function code hard to understand. It made refactoring code harder than it needed to be. It made declaring new variables in nested scopes a problem because a reference to the outer name may go unnoticed, and a weird bug introduced.

Some judgements about why shadowing of globals is allowed:

1. It's just a bad idea in general to name globals with short names, and locals with long names. `x` should never be a global name. By following conventional coding practice, this should never be a problem.

2. It's just a bad idea in general to have many global variable declarations.

3. Having existing function declarations prevent changes to global names seems backwards.

4. I've never seen a bug from shadowing a global variable.

5. Anti-shadowing was a new concept at the time, and I decided to be a bit modest with it.

Overall, experience shows the proscription of shadowing has been a resounding success.

On 8/16/20 7:34 PM, Walter Bright wrote:
> On 8/14/2020 5:51 AM, Steven Schveighoffer wrote:
>> On 8/14/20 4:58 AM, Per Nordlöw wrote:
>>> Shouldn't this code give a shadowing error?
>>>
>>> int x;                          // global?
>>>
>>> void test() @safe nothrow @nogc
>>> {
>>>      int x = x;                  // shouldn't this give a shadowing warning?
>>> }
>>>
>>
>> I don't know if this is a hard-fast rule, but my view of the shadowing error is that if you can access the variable, it's not shadowing.
>> You can access the global via .x
> 
> It's a reasonable way to put it.
> 
> More to Per's query, the current behavior is not an accident nor a bug, it is deliberate.
> 
> It came about because in my long experience coding, I had repeated bugs due to shadowing of locals. It was difficult to tell, for longer functions, if a variable had already been declared. Even when done correctly, shadowing declarations made function code hard to understand. It made refactoring code harder than it needed to be. It made declaring new variables in nested scopes a problem because a reference to the outer name may go unnoticed, and a weird bug introduced.
> 
> Some judgements about why shadowing of globals is allowed:
> 
> 1. It's just a bad idea in general to name globals with short names, and locals with long names. `x` should never be a global name. By following conventional coding practice, this should never be a problem.
> 
> 2. It's just a bad idea in general to have many global variable declarations.
> 
> 3. Having existing function declarations prevent changes to global names seems backwards.
> 
> 4. I've never seen a bug from shadowing a global variable.
> 
> 5. Anti-shadowing was a new concept at the time, and I decided to be a bit modest with it.
> 
> Overall, experience shows the proscription of shadowing has been a resounding success.

TDPL has a good explanation of why local shadowing is disallowed and non-local shadowing is allowed. In parts it goes similar to Walter's explanation, but the crux of it is different:

Introducing or changing a global name should not break unrelated code at a distance.

That's it.