[PATCH] D148712: [clang] Diagnose shadowing of lambda's template parameter by a capture

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp:7
+                                     // expected-note {{variable 'x' is explicitly captured here}}
+  auto h = [y = 0]<typename y>(y) { return 0; };  // expected-error {{declaration of 'y' shadows template parameter}} \
+                                                  // expected-note {{template parameter is declared here}}
----------------
erichkeane wrote:
> Fznamznon wrote:
> > shafik wrote:
> > > I don't know if shadowing is the correct term to use here. The wording simply says they can't have the same name. I think the diagnostic should say something similar. 
> > Well, the example with capture and a parameter uses shadowing term, so I just followed the same approach.
> > 
> > If we say something like "explicitly captured entity and template parameter can't have the same name", does it make sense to emit a note "captured here" for the capture with conflicting name?
> I think the 'shadows' is an appropriate as it matches what we do in normal template cases:
> 
> ```
> template<typename T>
> int foo(int T) {
>     int T = 5;
> }
> 
> <source>:3:13: error: declaration of 'T' shadows template parameter
> int foo(int T) {
>             ^
> <source>:2:19: note: template parameter is declared here
> template<typename T>
>                   ^
> <source>:4:9: error: redefinition of 'T'
>     int T = 5;
>         ^
> <source>:3:13: note: previous definition is here
> int foo(int T) {
> ```
So [shadowing](https://en.wikipedia.org/wiki/Variable_shadowing) is synonymous w/ name hiding which is not really the case here. I will concede it is consistent with how we are using elsewhere but this PR is probably not the place to change it. So I will drop the objection and probably file a bug report. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148712/new/

https://reviews.llvm.org/D148712