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

[Fznamznon]

Summary

expr.prim.lambda.capture p5 says:
If an identifier in a capture appears as the declarator-id of a parameter of
the lambda-declarator's parameter-declaration-clause or as the name of a
template parameter of the lambda-expression's template-parameter-list,
the program is ill-formed.
and also has the following example:

auto h = [y = 0]<typename y>(y) { return 0; };

which now results in

error: declaration of 'y' shadows template parameter
  auto l1 = [y = 0]<typename y>(y) { return 0; };
             ^
note: template parameter is declared here
  auto l1 = [y = 0]<typename y>(y) { return 0; };
                             ^

Fixes https://github.com/llvm/llvm-project/issues/61105

[shafik]

clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp

8

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.

[Fznamznon]

clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp

8

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?

[erichkeane]

clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp

8

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

[shafik]

clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp

8

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

[cor3ntin]

clang/lib/Sema/SemaLambda.cpp

1368

clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp

8

Well, if it was allowed, it would shadow and it's not allowed.
I think the message is clear for user - maybe we could ad "which is not allowed" or something, so that it isn't confused with a shadow warning.

[shafik]

clang/lib/Sema/SemaLambda.cpp

1381

It is really a shame that this is just different enough that we can't turn CheckRedefinition into a generic lambda and reuse it here. The code duplication is very unfortunate.

clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp

8

Also note the example you gave Erich the ordering is different, the template parameter comes before the function parameter but here it is vice versa, so it feels kind of awkward to say it is shadowing a name that comes after it.

clang/test/SemaCXX/warn-shadow-in-lambdas.cpp

160

Can we also get a test that verifies this is ok []<typename y>(y) { return 0; } and we obtain no diagnostic.

[Fznamznon]

Rebase, mention C++23, add a test case

[Fznamznon]

clang/lib/Sema/SemaLambda.cpp

1381

What I could do here is modify err_parameter_shadow_capture message so it says either a lambda parameter cannot shadow an explicitly captured entity or a lambda template parameter cannot shadow an explicitly captured entity and make CheckRedefinition accept a NamedDecl and a flag whether we're diagnosing a template parameter. Then CheckRedefinition can be reused, resolving the concerns about code duplication and the fact that explicit capture comes before the template parameter. That would probably be inconsistent with how the templates are diagnosed now though. WDYT?

[Fznamznon]

Modify comment

[cor3ntin]

Thanks, this looks good to me now!

[shafik]

LGTM

[Fznamznon]

Rebase to double check that pre-commit fail is unrelated, add a release note

[Fznamznon]

Rebase

clang/docs/ReleaseNotes.rst

- Clang now avoids unnecessary diagnostic warnings for obvious expressions in
   the case of binary operators with logical OR operations.
   (`#57906 <https://github.com/llvm/llvm-project/issues/57906>`_)
 - Clang's "static assertion failed" diagnostic now points to the static assertion
   expression instead of pointing to the ``static_assert`` token.
   (`#61951 <https://github.com/llvm/llvm-project/issues/61951>`_)
 - ``-Wformat`` now recognizes ``%lb`` for the ``printf``/``scanf`` family of
   functions.
   (`#62247: <https://github.com/llvm/llvm-project/issues/62247>`_).
+- Clang now diagnoses shadowing of lambda's template parameter by a capture.
+  (`#61105: <https://github.com/llvm/llvm-project/issues/61105>`_).
 
 Bug Fixes in This Version
 -------------------------
 
 - Fix segfault while running clang-rename on a non existing file.
   (`#36471 <https://github.com/llvm/llvm-project/issues/36471>`_)
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates.

clang/lib/Sema/SemaLambda.cpp

void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
   for (ParmVarDecl *P : Params) {
     if (!P->getIdentifier())
       continue;
     if (CheckRedefinition(P))
       CheckShadow(CurScope, P);
     PushOnScopeChains(P, CurScope);
   }
 
+  // C++23 [expr.prim.lambda.capture]p5:
+  // If an identifier in a capture appears as the declarator-id of a parameter
+  // of the lambda-declarator's parameter-declaration-clause or as the name of a
+  // template parameter of the lambda-expression's template-parameter-list, the
+  // program is ill-formed.
+  TemplateParameterList *TemplateParams =
+      getGenericLambdaTemplateParameterList(LSI, *this);
+  if (TemplateParams) {
+    for (const auto *TP : TemplateParams->asArray()) {
+      if (!TP->getIdentifier())
+        continue;
+      for (const auto &Capture : Intro.Captures) {
+        if (Capture.Id == TP->getIdentifier()) {
+          Diag(Capture.Loc, diag::err_template_param_shadow) << Capture.Id;
+          Diag(TP->getLocation(), diag::note_template_param_here);
+        }
+      }
+    }
+  }
+
   // C++20: dcl.decl.general p4:
   // The optional requires-clause ([temp.pre]) in an init-declarator or
   // member-declarator shall be present only if the declarator declares a
   // templated function ([dcl.fct]).
   if (Expr *TRC = Method->getTrailingRequiresClause()) {
     // [temp.pre]/8:
     // An entity is templated if it is
     // - a template,

clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp

+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+void f() {
+  int x = 0;
+  auto g = [x](int x) { return 0; }; // expected-error {{a lambda parameter cannot shadow an explicitly captured entity}} \
+                                     // 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}}
+
+}

clang/test/SemaCXX/warn-shadow-in-lambdas.cpp

 // RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -Wshadow -D AVOID %s
 // RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -Wshadow -Wshadow-uncaptured-local %s
 // RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -Wshadow-all %s
 // RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only -Wshadow-all %s
+// RUN: %clang_cc1 -std=c++20 -verify -fsyntax-only -Wshadow-all %s
 
 void foo(int param) { // expected-note 1+ {{previous declaration is here}}
   int var = 0; // expected-note 1+ {{previous declaration is here}}
 
   // Avoid warnings for variables that aren't implicitly captured.
   {
 #ifdef AVOID
     auto f1 = [=] { int var = 1; };  // no warning

 
 void avoidWarningWhenRedefining() {
   int a = 1;
   auto l = [b = a] { // expected-note {{previous definition is here}}
     // Don't warn on redefinitions.
     int b = 0; // expected-error {{redefinition of 'b'}}
   };
 }
+
+namespace GH61105 {
+void f() {
+  int y = 0;
+  int x = 0;
+#if __cplusplus >= 202002L
+  auto l1 = [y]<typename y>(y) { return 0; }; // expected-error {{declaration of 'y' shadows template parameter}} \
+                                              // expected-note {{template parameter is declared here}}
+  auto l2 = [=]<typename y>() { int a = y; return 0; }; // expected-error {{'y' does not refer to a value}} \
+                                                        // expected-note {{declared here}}
+  auto l3 = [&, y]<typename y, typename>(y) { int a = x; return 0; }; // expected-error {{declaration of 'y' shadows template parameter}} \
+                                                                      // expected-note {{template parameter is declared here}}
+  auto l4 = [x, y]<typename y, int x>() { return 0; }; // expected-error {{declaration of 'y' shadows template parameter}} \
+                                                       // expected-error {{declaration of 'x' shadows template parameter}} \
+                                                       // expected-note 2{{template parameter is declared here}}
+  auto l5 = []<typename y>(y) { return 0; }; // No diagnostic
+#endif
+}
+}