diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 41bec2666f939..bdf8334f78cea 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -316,6 +316,11 @@ Bug Fixes in This Version - Builtin elementwise operators now accept vector arguments that have different qualifiers on their elements. For example, vector of 4 ``const float`` values and vector of 4 ``float`` values. (#GH155405) +- Fixed inconsistent shadow warnings for lambda capture of structured bindings. + Previously, ``[val = val]`` (regular parameter) produced no warnings with ``-Wshadow`` + while ``[a = a]`` (where ``a`` is from ``auto [a, b] = std::make_pair(1, 2)``) + incorrectly produced warnings. Both cases now consistently show no warnings with + ``-Wshadow`` and show uncaptured-local warnings with ``-Wshadow-all``. (#GH68605) - Fixed a failed assertion with a negative limit parameter value inside of ``__has_embed``. (#GH157842) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 2b0ddb584c37e..45cfb66996ce6 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8395,7 +8395,7 @@ static ShadowedDeclKind computeShadowedDeclKind(const NamedDecl *ShadowedDecl, /// Return the location of the capture if the given lambda captures the given /// variable \p VD, or an invalid source location otherwise. static SourceLocation getCaptureLocation(const LambdaScopeInfo *LSI, - const VarDecl *VD) { + const ValueDecl *VD) { for (const Capture &Capture : LSI->Captures) { if (Capture.isVariableCapture() && Capture.getVariable() == VD) return Capture.getLocation(); @@ -8492,7 +8492,9 @@ 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 (const auto *VD = dyn_cast(ShadowedDecl)) { + // Handle both VarDecl and BindingDecl in lambda contexts + if (isa(ShadowedDecl)) { + const auto *VD = cast(ShadowedDecl); const auto *LSI = cast(getCurFunction()); if (RD->getLambdaCaptureDefault() == LCD_None) { // Try to avoid warnings for lambdas with an explicit capture @@ -8521,18 +8523,27 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *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. - 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. - if (!isa(ParentDC) && !isa(ParentDC) && - !isLambdaCallOperator(ParentDC)) { - return; + // Apply scoping logic to both VarDecl and BindingDecl with local storage + if (isa(ShadowedDecl)) { + bool HasLocalStorage = false; + if (const auto *VD = dyn_cast(ShadowedDecl)) + HasLocalStorage = VD->hasLocalStorage(); + else if (const auto *BD = dyn_cast(ShadowedDecl)) + HasLocalStorage = + cast(BD->getDecomposedDecl())->hasLocalStorage(); + + if (HasLocalStorage) { + // A variable can't shadow a local variable or binding 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. + if (!isa(ParentDC) && !isa(ParentDC) && + !isLambdaCallOperator(ParentDC)) + return; } } } @@ -8579,7 +8590,8 @@ void Sema::DiagnoseShadowingLambdaDecls(const LambdaScopeInfo *LSI) { const NamedDecl *ShadowedDecl = Shadow.ShadowedDecl; // Try to avoid the warning when the shadowed decl isn't captured. const DeclContext *OldDC = ShadowedDecl->getDeclContext(); - if (const auto *VD = dyn_cast(ShadowedDecl)) { + if (isa(ShadowedDecl)) { + const auto *VD = cast(ShadowedDecl); SourceLocation CaptureLoc = getCaptureLocation(LSI, VD); Diag(Shadow.VD->getLocation(), CaptureLoc.isInvalid() ? diag::warn_decl_shadow_uncaptured_local diff --git a/clang/test/SemaCXX/PR68605.cpp b/clang/test/SemaCXX/PR68605.cpp new file mode 100644 index 0000000000000..97eb858b77246 --- /dev/null +++ b/clang/test/SemaCXX/PR68605.cpp @@ -0,0 +1,72 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -std=c++20 -Wshadow %s +// RUN: %clang_cc1 -verify=all -fsyntax-only -std=c++20 -Wshadow-all %s + +// Test for issue #68605: Inconsistent shadow warnings for lambda capture of structured bindings. +// +// The issue was that structured binding lambda captures were incorrectly classified +// as regular shadow warnings (shown with -Wshadow) while regular parameter captures +// were classified as uncaptured-local warnings (shown only with -Wshadow-all). +// +// This test validates that both VarDecl and BindingDecl lambda captures now +// behave consistently: no warnings with -Wshadow, but uncaptured-local warnings +// with -Wshadow-all. + +namespace std { + template T&& move(T&& t) { return static_cast(t); } +} + +namespace issue_68605 { + +// Simple pair-like struct for testing +struct Pair { + int first; + int second; + Pair(int f, int s) : first(f), second(s) {} +}; + +// Test case 1: Regular parameter - consistent behavior +void foo1(Pair val) { // all-note {{previous declaration is here}} + [val = std::move(val)](){}(); // all-warning {{declaration shadows a local variable}} +} + +// Test case 2: Structured binding - now consistent with regular parameter +void foo2(Pair val) { + auto [a,b] = val; // all-note {{previous declaration is here}} + [a = std::move(a)](){}(); // all-warning {{declaration shadows a structured binding}} +} + +// Test case 3: Multiple captures showing consistent behavior +void foo3() { + Pair data{42, 100}; + auto [id, value] = data; // all-note 2{{previous declaration is here}} + + // Both show consistent uncaptured-local warnings with -Wshadow-all + auto lambda1 = [id = id](){ return id; }; // all-warning {{declaration shadows a structured binding}} + auto lambda2 = [value = value](){ return value; }; // all-warning {{declaration shadows a structured binding}} +} + +// Test case 4: Mixed scenario showing consistent behavior +void foo4() { + int regular_var = 10; // all-note {{previous declaration is here}} + Pair pair_data{1, 2}; + auto [x, y] = pair_data; // all-note 2{{previous declaration is here}} + + // All captures now show consistent uncaptured-local warnings with -Wshadow-all + auto lambda1 = [regular_var = regular_var](){}; // all-warning {{declaration shadows a local variable}} + auto lambda2 = [x = x](){}; // all-warning {{declaration shadows a structured binding}} + auto lambda3 = [y = y](){}; // all-warning {{declaration shadows a structured binding}} +} + +// Test case 5: Ensure we don't break existing shadow detection for actual shadowing +void foo5() { + int outer = 5; // expected-note {{previous declaration is here}} all-note {{previous declaration is here}} + auto [a, b] = Pair{1, 2}; // expected-note {{previous declaration is here}} all-note {{previous declaration is here}} + + // This SHOULD still warn - it's actual shadowing within the lambda body + auto lambda = [outer, a](){ // expected-note {{variable 'outer' is explicitly captured here}} all-note {{variable 'outer' is explicitly captured here}} expected-note {{variable 'a' is explicitly captured here}} all-note {{variable 'a' is explicitly captured here}} + int outer = 10; // expected-warning {{declaration shadows a local variable}} all-warning {{declaration shadows a local variable}} + int a = 20; // expected-warning {{declaration shadows a structured binding}} all-warning {{declaration shadows a structured binding}} + }; +} + +} // namespace issue_68605 diff --git a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp index d54b394df4eb8..2388c5f16e4ca 100644 --- a/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp +++ b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp @@ -258,10 +258,15 @@ struct S { }; int foo() { - auto [a] = S{0}; // expected-note {{previous}} \ - // cxx14-warning {{decomposition declarations are a C++17 extension}} +#ifdef AVOID + auto [a] = S{0}; // cxx14-warning {{decomposition declarations are a C++17 extension}} + [a = a] () { // No warning with basic -Wshadow due to uncaptured-local classification + }(); +#else + auto [a] = S{0}; // cxx14-warning {{decomposition declarations are a C++17 extension}} expected-note {{previous declaration is here}} [a = a] () { // expected-warning {{declaration shadows a structured binding}} }(); +#endif } }