Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8410,6 +8410,7 @@ NamedDecl *Sema::getShadowedDeclaration(const VarDecl *D,
return nullptr;

NamedDecl *ShadowedDecl = R.getFoundDecl();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unrelated.

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unrelated.

Restored at 27fc9b7

return isa<VarDecl, FieldDecl, BindingDecl>(ShadowedDecl) ? ShadowedDecl
: nullptr;
}
Expand Down Expand Up @@ -8479,6 +8480,7 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
if (isa<VarDecl>(D) && NewDC && isa<CXXMethodDecl>(NewDC)) {
if (const auto *RD = dyn_cast<CXXRecordDecl>(NewDC->getParent())) {
if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) {
// Handle lambda capture logic for both VarDecl and BindingDecl
if (const auto *VD = dyn_cast<VarDecl>(ShadowedDecl)) {
const auto *LSI = cast<LambdaScopeInfo>(getCurFunction());
if (RD->getLambdaCaptureDefault() == LCD_None) {
Expand All @@ -8496,6 +8498,21 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
->ShadowingDecls.push_back({D, VD});
return;
}
} else if (isa<BindingDecl>(ShadowedDecl)) {
// Apply lambda capture logic only when D is actually a lambda capture
if (isa<VarDecl>(D) && cast<VarDecl>(D)->isInitCapture()) {
if (RD->getLambdaCaptureDefault() == LCD_None) {
// BindingDecls cannot be explicitly captured, so always treat as
// uncaptured
WarningDiag = diag::warn_decl_shadow_uncaptured_local;
} else {
// Same deferred handling as VarDecl
cast<LambdaScopeInfo>(getCurFunction())
->ShadowingDecls.push_back({D, ShadowedDecl});
return;
}
}
// For non-init-capture cases, fall through to regular shadow logic
}
if (isa<FieldDecl>(ShadowedDecl)) {
// If lambda can capture this, then emit default shadowing warning,
Expand All @@ -8508,10 +8525,17 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
return;
}
}
if (const auto *VD = dyn_cast<VarDecl>(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.
// Apply scoping logic to both VarDecl and BindingDecl
bool shouldApplyScopingLogic = false;
if (const auto *VD = dyn_cast<VarDecl>(ShadowedDecl)) {
shouldApplyScopingLogic = VD->hasLocalStorage();
} else if (isa<BindingDecl>(ShadowedDecl)) {
shouldApplyScopingLogic = true;
}

if (shouldApplyScopingLogic) {
// 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)) {
Expand Down Expand Up @@ -8577,6 +8601,12 @@ void Sema::DiagnoseShadowingLambdaDecls(const LambdaScopeInfo *LSI) {
Diag(CaptureLoc, diag::note_var_explicitly_captured_here)
<< Shadow.VD->getDeclName() << /*explicitly*/ 0;
Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
} else if (isa<BindingDecl>(ShadowedDecl)) {
// BindingDecls cannot be explicitly captured, so always uncaptured-local
Diag(Shadow.VD->getLocation(), diag::warn_decl_shadow_uncaptured_local)
<< Shadow.VD->getDeclName()
<< computeShadowedDeclKind(ShadowedDecl, OldDC) << OldDC;
Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
} else if (isa<FieldDecl>(ShadowedDecl)) {
Diag(Shadow.VD->getLocation(),
LSI->isCXXThisCaptured() ? diag::warn_decl_shadow
Expand Down
72 changes: 72 additions & 0 deletions clang/test/SemaCXX/PR68605.cpp
Original file line number Diff line number Diff line change
@@ -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<typename T> T&& move(T&& t) { return static_cast<T&&>(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}}
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a newline at the end of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed at ef25bd2

9 changes: 7 additions & 2 deletions clang/test/SemaCXX/warn-shadow-in-lambdas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

}