Hi all,

I just discovered a silly programming error on my side. Stripped down, it looks like this:
```
template foo(T) {
    pragma(msg, __LINE__, ": ", T);

    // This should not have the "(T)"
    void foo(T)(T i) {
        pragma(msg, __LINE__, ": ", T);
    }
}

void main()
{
    foo!long(1);
}
```
which outputs
2: long
5: int
(https://d.godbolt.org/z/8K6sjc)

I made a template inside a template, where I intended to make just one eponymous template.

What do you think about adding a warning about shadowing of an outer template parameter by an inner template parameter name?

cheers,
  Johan

On Wed, Aug 05, 2020 at 11:12:53AM +0000, Johan via Digitalmars-d wrote: [...]
> What do you think about adding a warning about shadowing of an outer template parameter by an inner template parameter name?
[...]

Probably a good idea!


T

-- 
The only difference between male factor and malefactor is just a little emptiness inside.

On Wednesday, 5 August 2020 at 11:12:53 UTC, Johan wrote:
> What do you think about adding a warning about shadowing of an outer template parameter by an inner template parameter name?
>
> cheers,
>   Johan

Not a fan. There are cases where I've done this intentionally--not with eponymous template members, but with private helper templates:

template foo(T) {
    enum helper(T) = ...; // error?
    void foo(T i) {
        static if (helper!T) ...;
        else ...;
    }
}

On 8/5/20 12:10 PM, Paul Backus wrote:
> On Wednesday, 5 August 2020 at 11:12:53 UTC, Johan wrote:
>> What do you think about adding a warning about shadowing of an outer template parameter by an inner template parameter name?
>>
> 
> Not a fan. There are cases where I've done this intentionally--not with eponymous template members, but with private helper templates:
> 
> template foo(T) {
>      enum helper(T) = ...; // error?
>      void foo(T i) {
>          static if (helper!T) ...;
>          else ...;
>      }
> }

I've intentionally done that, and even though it worked, rewrote the template parameter names because it's confusing to read.

It would have to be a long deprecation cycle.

I would be in favor of the change.

-Steve

On Wednesday, 5 August 2020 at 16:22:48 UTC, Steven Schveighoffer wrote:
> On 8/5/20 12:10 PM, Paul Backus wrote:
>> 
>> Not a fan. There are cases where I've done this intentionally--not with eponymous template members, but with private helper templates:
>> 
>> template foo(T) {
>>      enum helper(T) = ...; // error?
>>      void foo(T i) {
>>          static if (helper!T) ...;
>>          else ...;
>>      }
>> }
>
> I've intentionally done that, and even though it worked, rewrote the template parameter names because it's confusing to read.
>
> It would have to be a long deprecation cycle.
>
> I would be in favor of the change.
>
> -Steve

"Confusing to read" does not justify a compiler warning. The compiler accepts lots of code that's confusing to read.

The appropriate place for a rule like this is in a style guide or a configurable linter.

(This would also interact very badly with string-based code generation, since there's no reliable way to generate unique identifiers at compile time.)

On Wed, Aug 05, 2020 at 04:35:12PM +0000, Paul Backus via Digitalmars-d wrote:
> [...] there's no reliable way to generate unique identifiers at compile time.)

Sure there is. Well, depending on what you need said identifiers for, that is.  Given a unique tuple, a template instantiated with that tuple is a unique identifier for that tuple.

I've used this fact to generate unique identifiers for temporaries in static foreach loops, since static foreach does not introduce a new scope, so all iterations share the same namespace, and any temporaries you might declare will collide with other iterations and fail with a compile error. To work around this, I wrap the desired declaration(s) in an auxiliary template that gets instantiated with the index variable of the static foreach, thus guaranteeing uniqueness:

	struct TmpStruct(int iter) {
		...
	}

	int someAcc;
	switch (someVar) {
		static foreach (i; 0 .. 99) {
			case i:
				auto tmp = TmpStruct!i(...);
				doSomething(tmp);
				break;
		}
	}


T

-- 
Long, long ago, the ancient Chinese invented a device that lets them see through walls. It was called the "window".

On 8/5/20 12:35 PM, Paul Backus wrote:
> On Wednesday, 5 August 2020 at 16:22:48 UTC, Steven Schveighoffer wrote:
>> On 8/5/20 12:10 PM, Paul Backus wrote:
>>>
>>> Not a fan. There are cases where I've done this intentionally--not with eponymous template members, but with private helper templates:
>>>
>>> template foo(T) {
>>>      enum helper(T) = ...; // error?
>>>      void foo(T i) {
>>>          static if (helper!T) ...;
>>>          else ...;
>>>      }
>>> }
>>
>> I've intentionally done that, and even though it worked, rewrote the template parameter names because it's confusing to read.
>>
>> It would have to be a long deprecation cycle.
>>
>> I would be in favor of the change.
>>
> 
> "Confusing to read" does not justify a compiler warning. The compiler accepts lots of code that's confusing to read.

That's not a justification, just my personal experience. I read my code again and think, "wait, what T am I talking about? I'll just change it to U."

For the same reason, I don't like to name inner struct members the same as any function variables, even though that's allowed.

In essence, this is no different than erroring on shadowing of function variables, which could also be allowed, but saves a lot of trouble. You can look at it more as a "hey D, you have this position on variable names, but not on template parameter names, why the inconsistency?"

Note that I think it purely should be for nested template *parameters*, not for other names.

> (This would also interact very badly with string-based code generation, since there's no reliable way to generate unique identifiers at compile time.)
Somehow people cope with this for shadowed variable names just fine.

-Steve

On Wednesday, 5 August 2020 at 16:57:49 UTC, Steven Schveighoffer wrote:
> In essence, this is no different than erroring on shadowing of function variables, which could also be allowed, but saves a lot of trouble. You can look at it more as a "hey D, you have this position on variable names, but not on template parameter names, why the inconsistency?"

The case you point out here is the exception, not the rule. D allows shadowing for everything *except* local variables in functions. The most consistent thing to do would be to allow it everywhere, period; presumably an exception was made because that specific type of shadowing was thought to be particularly error-prone.

Note that the exception does not apply to *parameters* of nested functions, which is the analogous case here:

void foo(int x) {
    void bar(int x) {} // ok
}

On 8/5/20 1:27 PM, Paul Backus wrote:
> On Wednesday, 5 August 2020 at 16:57:49 UTC, Steven Schveighoffer wrote:
>> In essence, this is no different than erroring on shadowing of function variables, which could also be allowed, but saves a lot of trouble. You can look at it more as a "hey D, you have this position on variable names, but not on template parameter names, why the inconsistency?"
> 
> The case you point out here is the exception, not the rule. D allows shadowing for everything *except* local variables in functions. The most consistent thing to do would be to allow it everywhere, period; presumably an exception was made because that specific type of shadowing was thought to be particularly error-prone.

One reason shadowing of variables is not allowed, but shadowing of module-level symbols is fine is because you can explicitly refer to the module-level symbols by prepending a dot.

I haven't thought completely through it, but if I could, I'd ban all shadowing of anything that could not be accessed via some explicit scope expression.

> 
> Note that the exception does not apply to *parameters* of nested functions, which is the analogous case here:
> 
> void foo(int x) {
>      void bar(int x) {} // ok
> }

Yes you are right of course. I happen to believe it should require a difference here as well. Again, in my own code, I avoid this case.

I still believe that a) it's very error prone to define nested templates with the same parameter names as the parameters of the outer template, and b) the burden of requiring everything to have a unique name would be minimal.

I happen to do this anyway when I remember. I would appreciate the compiler helping me out when I don't. Hence, I'm in favor of such a proposal.

-Steve

On Wednesday, 5 August 2020 at 17:54:47 UTC, Steven Schveighoffer wrote:
> I happen to do this anyway when I remember. I would appreciate the compiler helping me out when I don't. Hence, I'm in favor of such a proposal.
>
> -Steve

I also have coding style preferences that the compiler doesn't help me with. It doesn't follow that I'm in favor of making them into compiler warnings. :)