Skip to content

Commit

Permalink
[backport][clang] Avoid -Wshadow warning when init-capture named same
Browse files Browse the repository at this point in the history
 as class field

Shadowing warning doesn't make much sense since field is not available
in lambda's body without capturing this.

Fixes llvm/llvm-project#71976
  • Loading branch information
wangqiang committed Apr 12, 2024
1 parent af855a7 commit 5491555
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 31 deletions.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,9 @@ Bug Fixes in This Version
``thread_local`` instead of ``_Thread_local``.
Fixes (`#70068 <https://github.com/llvm/llvm-project/issues/70068>`_) and
(`#69167 <https://github.com/llvm/llvm-project/issues/69167>`_)
- Clang's ``-Wshadow`` no longer warns when an init-capture is named the same as
a class field unless the lambda can capture this.
Fixes (`#71976 <https://github.com/llvm/llvm-project/issues/71976>`_)

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/Sema/ScopeInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -915,8 +915,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<ShadowedOuterDecl, 4> ShadowingDecls;

Expand Down
73 changes: 47 additions & 26 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8269,28 +8269,40 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,

unsigned WarningDiag = diag::warn_decl_shadow;
SourceLocation CaptureLoc;
if (isa<VarDecl>(D) && isa<VarDecl>(ShadowedDecl) && NewDC &&
isa<CXXMethodDecl>(NewDC)) {
if (isa<VarDecl>(D) && NewDC && isa<CXXMethodDecl>(NewDC)) {
if (const auto *RD = dyn_cast<CXXRecordDecl>(NewDC->getParent())) {
if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) {
if (RD->getLambdaCaptureDefault() == LCD_None) {
// Try to avoid warnings for lambdas with an explicit capture list.
if (const auto *VD = dyn_cast<VarDecl>(ShadowedDecl)) {
const auto *LSI = cast<LambdaScopeInfo>(getCurFunction());
// Warn only when the lambda captures the shadowed decl explicitly.
CaptureLoc = getCaptureLocation(LSI, cast<VarDecl>(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<LambdaScopeInfo>(getCurFunction())
->ShadowingDecls.push_back({D, VD});
return;
}
}
if (isa<FieldDecl>(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<LambdaScopeInfo>(getCurFunction())
->ShadowingDecls.push_back(
{cast<VarDecl>(D), cast<VarDecl>(ShadowedDecl)});
->ShadowingDecls.push_back({D, ShadowedDecl});
return;
}
}

if (cast<VarDecl>(ShadowedDecl)->hasLocalStorage()) {
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.
for (DeclContext *ParentDC = NewDC;
Expand Down Expand Up @@ -8337,19 +8349,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<VarDecl>(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.isValid())
Diag(CaptureLoc, diag::note_var_explicitly_captured_here)
<< Shadow.VD->getDeclName() << /*explicitly*/ 0;
Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
} else if (isa<FieldDecl>(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);
}
}
}

Expand Down
92 changes: 89 additions & 3 deletions clang/test/SemaCXX/warn-shadow-in-lambdas.cpp
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -179,3 +179,89 @@ void f() {
#endif
}
}

namespace GH71976 {
#ifdef AVOID
struct A {
int b = 5;
int foo() {
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 -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}}
}
};

struct E {
int b = 5;
int foo() {
return [a = b]() { // expected-note {{previous}}
return [=, a = a]() { // expected-warning {{shadows a local}}
return a;
}();
}();
}
};

#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}}
}();
}

}

0 comments on commit 5491555

Please sign in to comment.