From 169d962b64b8ae242c3a6d332677296cf7503839 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" Date: Tue, 5 Dec 2023 10:28:44 -0800 Subject: [PATCH 1/6] [clang] Avoid -Wshadow warning when init-capture named same as class field Shadowing warning doesn't make much sense since field is not available in lambda's body without capturing this. Fixes https://github.com/llvm/llvm-project/issues/71976 --- clang/docs/ReleaseNotes.rst | 3 +++ clang/lib/Sema/SemaDecl.cpp | 8 +++++--- clang/test/SemaCXX/warn-shadow-in-lambdas.cpp | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 828dd10e3d6db9..7ac81e16492d1f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -655,6 +655,9 @@ Bug Fixes in This Version Fixes (`#64467 `_) - Clang's ``-Wchar-subscripts`` no longer warns on chars whose values are known non-negative constants. Fixes (`#18763 `_) +- Clang's ``-Wshadow`` no longer warns when init-capture named same as class + field. + Fixes (`#71976 `_) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index f12424d33b7da2..65d095b2431ddd 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8395,10 +8395,11 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, unsigned WarningDiag = diag::warn_decl_shadow; SourceLocation CaptureLoc; - if (isa(D) && isa(ShadowedDecl) && NewDC && - isa(NewDC)) { + if (isa(D) && NewDC && isa(NewDC)) { if (const auto *RD = dyn_cast(NewDC->getParent())) { if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) { + if (!isa(ShadowedDecl)) + return; if (RD->getLambdaCaptureDefault() == LCD_None) { // Try to avoid warnings for lambdas with an explicit capture list. const auto *LSI = cast(getCurFunction()); @@ -8416,7 +8417,8 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, } } - if (cast(ShadowedDecl)->hasLocalStorage()) { + if (const auto *VD = dyn_cast(ShadowedDecl); + VD && VD->hasLocalStorage()) { // A variable can't shadow a local variable in an enclosing scope, if // they are separated by a non-capturing declaration context. for (DeclContext *ParentDC = NewDC; diff --git a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp index bda6a65c02168b..58af7a2e65c559 100644 --- a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp +++ b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp @@ -179,3 +179,21 @@ void f() { #endif } } + +namespace GH71976 { +struct A { + int b = 5; + int foo() { + return [b = b]() { return b; }(); + } +}; + +struct B { + int a; + void foo() { + auto b = [a = this->a] { + + }; + } +}; +} From 3798e1f25a8d812c082ac5815c490eb9c7f67e62 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" Date: Wed, 6 Dec 2023 02:24:59 -0800 Subject: [PATCH 2/6] Apply comments --- clang/docs/ReleaseNotes.rst | 4 ++-- clang/lib/Sema/SemaDecl.cpp | 1 + clang/test/SemaCXX/warn-shadow-in-lambdas.cpp | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 2d1f3ed22cf2ea..f5e0d4fa397372 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -661,8 +661,8 @@ Bug Fixes in This Version - Fixed false positive error emitted when templated alias inside a class used private members of the same class. Fixes (`#41693 `_) -- Clang's ``-Wshadow`` no longer warns when init-capture named same as class - field. +- Clang's ``-Wshadow`` no longer warns when an init-capture is named the same as + a class field. Fixes (`#71976 `_) Bug Fixes to Compiler Builtins diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 65d095b2431ddd..f590d1b3ade819 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8398,6 +8398,7 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, if (isa(D) && NewDC && isa(NewDC)) { if (const auto *RD = dyn_cast(NewDC->getParent())) { if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) { + // If it is not VarDecl then it can not shadow. if (!isa(ShadowedDecl)) return; if (RD->getLambdaCaptureDefault() == LCD_None) { diff --git a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp index 58af7a2e65c559..3008e9018f30b2 100644 --- a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp +++ b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp @@ -184,14 +184,14 @@ namespace GH71976 { struct A { int b = 5; int foo() { - return [b = b]() { return b; }(); + return [b = b]() { return b; }(); // no diagnostic, init-capture does not shadow b } }; struct B { int a; void foo() { - auto b = [a = this->a] { + auto b = [a = this->a] { // no diagnostic, init-capture does not shadow a }; } From 3f01cd31f5a1609402be40e5bf9d8a01b325e798 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" Date: Wed, 6 Dec 2023 09:21:38 -0800 Subject: [PATCH 3/6] Remove comment --- clang/lib/Sema/SemaDecl.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index f590d1b3ade819..65d095b2431ddd 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8398,7 +8398,6 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, if (isa(D) && NewDC && isa(NewDC)) { if (const auto *RD = dyn_cast(NewDC->getParent())) { if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) { - // If it is not VarDecl then it can not shadow. if (!isa(ShadowedDecl)) return; if (RD->getLambdaCaptureDefault() == LCD_None) { From 6b95d07e178a6e8764049108b263b15acbc13489 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" Date: Fri, 26 Jan 2024 08:18:19 -0800 Subject: [PATCH 4/6] Emit -Wshadow when captured this, otherwise emit -Wshadow-uncaptured-local --- clang/include/clang/Sema/ScopeInfo.h | 4 +- clang/lib/Sema/SemaDecl.cpp | 77 ++++++++++++------- clang/test/SemaCXX/warn-shadow-in-lambdas.cpp | 49 +++++++++++- 3 files changed, 96 insertions(+), 34 deletions(-) diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index 6eaa74382685ba..06e47eed4e93b6 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -925,8 +925,8 @@ class LambdaScopeInfo final : /// that were defined in parent contexts. Used to avoid warnings when the /// shadowed variables are uncaptured by this lambda. struct ShadowedOuterDecl { - const VarDecl *VD; - const VarDecl *ShadowedDecl; + const NamedDecl *VD; + const NamedDecl *ShadowedDecl; }; llvm::SmallVector ShadowingDecls; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index a2dee68815bae2..2f25c61657922a 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8399,34 +8399,44 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, if (isa(D) && NewDC && isa(NewDC)) { if (const auto *RD = dyn_cast(NewDC->getParent())) { if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) { - if (!isa(ShadowedDecl)) - return; - if (RD->getLambdaCaptureDefault() == LCD_None) { - // Try to avoid warnings for lambdas with an explicit capture list. + if (const auto *VD = dyn_cast(ShadowedDecl)) { const auto *LSI = cast(getCurFunction()); - // Warn only when the lambda captures the shadowed decl explicitly. - CaptureLoc = getCaptureLocation(LSI, cast(ShadowedDecl)); - if (CaptureLoc.isInvalid()) - WarningDiag = diag::warn_decl_shadow_uncaptured_local; - } else { - // Remember that this was shadowed so we can avoid the warning if the - // shadowed decl isn't captured and the warning settings allow it. + if (RD->getLambdaCaptureDefault() == LCD_None) { + // Try to avoid warnings for lambdas with an explicit capture + // list. Warn only when the lambda captures the shadowed decl + // explicitly. + CaptureLoc = getCaptureLocation(LSI, VD); + if (CaptureLoc.isInvalid()) + WarningDiag = diag::warn_decl_shadow_uncaptured_local; + } else { + // Remember that this was shadowed so we can avoid the warning if + // the shadowed decl isn't captured and the warning settings allow + // it. + cast(getCurFunction()) + ->ShadowingDecls.push_back({D, VD}); + return; + } + } + if (isa(ShadowedDecl)) { + // If lambda can capture this, then emit default shadowing warning, + // Otherwise it is not really a shadowing case since field is not + // available in lambda's body. + // At this point we don't know that lambda can capture this, so + // remember that this was shadowed and delay until we know. cast(getCurFunction()) - ->ShadowingDecls.push_back( - {cast(D), cast(ShadowedDecl)}); + ->ShadowingDecls.push_back({D, ShadowedDecl}); return; } } - if (const auto *VD = dyn_cast(ShadowedDecl); VD && VD->hasLocalStorage()) { - // A variable can't shadow a local variable in an enclosing scope, if - // they are separated by a non-capturing declaration context. + // A variable can't shadow a local variable in an enclosing scope, + // if they are separated by a non-capturing declaration context. for (DeclContext *ParentDC = NewDC; ParentDC && !ParentDC->Equals(OldDC); ParentDC = getLambdaAwareParentOfDeclContext(ParentDC)) { - // Only block literals, captured statements, and lambda expressions - // can capture; other scopes don't. + // Only block literals, captured statements, and lambda + // expressions can capture; other scopes don't. if (!isa(ParentDC) && !isa(ParentDC) && !isLambdaCallOperator(ParentDC)) { return; @@ -8470,19 +8480,28 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, /// when these variables are captured by the lambda. void Sema::DiagnoseShadowingLambdaDecls(const LambdaScopeInfo *LSI) { for (const auto &Shadow : LSI->ShadowingDecls) { - const VarDecl *ShadowedDecl = Shadow.ShadowedDecl; + const NamedDecl *ShadowedDecl = Shadow.ShadowedDecl; // Try to avoid the warning when the shadowed decl isn't captured. - SourceLocation CaptureLoc = getCaptureLocation(LSI, ShadowedDecl); const DeclContext *OldDC = ShadowedDecl->getDeclContext(); - Diag(Shadow.VD->getLocation(), CaptureLoc.isInvalid() - ? diag::warn_decl_shadow_uncaptured_local - : diag::warn_decl_shadow) - << Shadow.VD->getDeclName() - << computeShadowedDeclKind(ShadowedDecl, OldDC) << OldDC; - if (!CaptureLoc.isInvalid()) - Diag(CaptureLoc, diag::note_var_explicitly_captured_here) - << Shadow.VD->getDeclName() << /*explicitly*/ 0; - Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration); + if (const auto *VD = dyn_cast(ShadowedDecl)) { + SourceLocation CaptureLoc = getCaptureLocation(LSI, VD); + Diag(Shadow.VD->getLocation(), + CaptureLoc.isInvalid() ? diag::warn_decl_shadow_uncaptured_local + : diag::warn_decl_shadow) + << Shadow.VD->getDeclName() + << computeShadowedDeclKind(ShadowedDecl, OldDC) << OldDC; + if (!CaptureLoc.isInvalid()) + Diag(CaptureLoc, diag::note_var_explicitly_captured_here) + << Shadow.VD->getDeclName() << /*explicitly*/ 0; + Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration); + } else if (isa(ShadowedDecl)) { + Diag(Shadow.VD->getLocation(), + LSI->isCXXThisCaptured() ? diag::warn_decl_shadow + : diag::warn_decl_shadow_uncaptured_local) + << Shadow.VD->getDeclName() + << computeShadowedDeclKind(ShadowedDecl, OldDC) << OldDC; + Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration); + } } } diff --git a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp index 3008e9018f30b2..f712fd31fb5b5a 100644 --- a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp +++ b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp @@ -181,19 +181,62 @@ void f() { } namespace GH71976 { +#ifdef AVOID struct A { int b = 5; int foo() { - return [b = b]() { return b; }(); // no diagnostic, init-capture does not shadow b + return [b = b]() { return b; }(); // no -Wshadow diagnostic, init-capture does not shadow b due to not capturing this } }; struct B { int a; void foo() { - auto b = [a = this->a] { // no diagnostic, init-capture does not shadow a + auto b = [a = this->a] {}; // no -Wshadow diagnostic, init-capture does not shadow a due to not capturing his + } +}; - }; +struct C { + int b = 5; + int foo() { + return [a = b]() { + return [=, b = a]() { // no -Wshadow diagnostic, init-capture does not shadow b due to outer lambda + return b; + }(); + }(); } }; +#else +struct A { + int b = 5; // expected-note {{previous}} + int foo() { + return [b = b]() { return b; }(); // expected-warning {{declaration shadows a field}} + } +}; + +struct B { + int a; // expected-note {{previous}} + void foo() { + auto b = [a = this->a] {}; // expected-warning {{declaration shadows a field}} + } +}; + +struct C { + int b = 5; // expected-note {{previous}} + int foo() { + return [a = b]() { + return [=, b = a]() { // expected-warning {{declaration shadows a field}} + return b; + }(); + }(); + } +}; + +struct D { + int b = 5; // expected-note {{previous}} + int foo() { + return [b = b, this]() { return b; }(); // expected-warning {{declaration shadows a field}} + } +}; +#endif } From ea6bdd4ca56a5a40985c3f5c3359e5ce42541e52 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" Date: Fri, 9 Feb 2024 09:06:51 -0800 Subject: [PATCH 5/6] Apply comments --- clang/lib/Sema/SemaDecl.cpp | 10 +++++----- clang/test/SemaCXX/warn-shadow-in-lambdas.cpp | 12 ++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 6fa2337cc736c3..529fbcbfeff8e2 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8391,13 +8391,13 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, } if (const auto *VD = dyn_cast(ShadowedDecl); VD && VD->hasLocalStorage()) { - // A variable can't shadow a local variable in an enclosing scope, - // if they are separated by a non-capturing declaration context. + // A variable can't shadow a local variable in an enclosing scope, if + // they are separated by a non-capturing declaration context. for (DeclContext *ParentDC = NewDC; ParentDC && !ParentDC->Equals(OldDC); ParentDC = getLambdaAwareParentOfDeclContext(ParentDC)) { - // Only block literals, captured statements, and lambda - // expressions can capture; other scopes don't. + // Only block literals, captured statements, and lambda expressions + // can capture; other scopes don't. if (!isa(ParentDC) && !isa(ParentDC) && !isLambdaCallOperator(ParentDC)) { return; @@ -8451,7 +8451,7 @@ void Sema::DiagnoseShadowingLambdaDecls(const LambdaScopeInfo *LSI) { : diag::warn_decl_shadow) << Shadow.VD->getDeclName() << computeShadowedDeclKind(ShadowedDecl, OldDC) << OldDC; - if (!CaptureLoc.isInvalid()) + if (CaptureLoc.isValid()) Diag(CaptureLoc, diag::note_var_explicitly_captured_here) << Shadow.VD->getDeclName() << /*explicitly*/ 0; Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration); diff --git a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp index f712fd31fb5b5a..277355551bfcfb 100644 --- a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp +++ b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp @@ -206,6 +206,7 @@ struct C { }(); } }; + #else struct A { int b = 5; // expected-note {{previous}} @@ -238,5 +239,16 @@ struct D { return [b = b, this]() { return b; }(); // expected-warning {{declaration shadows a field}} } }; + +struct E { + int b = 5; + int foo() { + return [a = b]() { // expected-note {{previous}} + return [=, a = a]() { // expected-warning {{shadows a local}} + return a; + }(); + }(); + } +}; #endif } From 4b25344569f71f14195c452f14029a534a939ff0 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" Date: Fri, 9 Feb 2024 11:10:58 -0800 Subject: [PATCH 6/6] Add structured binding test. --- clang/test/SemaCXX/warn-shadow-in-lambdas.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp index 277355551bfcfb..d54b394df4eb84 100644 --- a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp +++ b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp @@ -1,6 +1,6 @@ -// 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++14 -verify=expected,cxx14 -fsyntax-only -Wshadow -D AVOID %s +// RUN: %clang_cc1 -std=c++14 -verify=expected,cxx14 -fsyntax-only -Wshadow -Wshadow-uncaptured-local %s +// RUN: %clang_cc1 -std=c++14 -verify=expected,cxx14 -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 @@ -250,5 +250,18 @@ struct E { }(); } }; + #endif + +struct S { + int a ; +}; + +int foo() { + auto [a] = S{0}; // expected-note {{previous}} \ + // cxx14-warning {{decomposition declarations are a C++17 extension}} + [a = a] () { // expected-warning {{declaration shadows a structured binding}} + }(); +} + }