From 175388c00d0a39838a6d61c54ca6256b34e37dab Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Wed, 9 Nov 2016 10:38:57 +0000 Subject: [PATCH] [Sema] Avoid -Wshadow warnings for shadowed variables that aren't captured by lambdas with an explicit capture list This commit avoids the -Wshadow warning for variables which shadow variables that aren't captured by lambdas with an explicit capture list. It provides an additional note that points to location of the explicit capture. The old behaviour is preserved with -Wshadow-all or -Wshadow-uncaptured-local. rdar://17135966 Differential Revision: https://reviews.llvm.org/D26278 llvm-svn: 286354 --- clang/include/clang/Basic/DiagnosticGroups.td | 4 +- clang/include/clang/Basic/DiagnosticSemaKinds.td | 5 ++ clang/lib/Sema/SemaDecl.cpp | 31 +++++++- clang/test/SemaCXX/warn-shadow-in-lambdas.cpp | 91 ++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 clang/test/SemaCXX/warn-shadow-in-lambdas.cpp diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 0b7e943..f20092c 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -348,12 +348,14 @@ def ShadowFieldInConstructorModified : DiagGroup<"shadow-field-in-constructor-mo def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-constructor", [ShadowFieldInConstructorModified]>; def ShadowIvar : DiagGroup<"shadow-ivar">; +def ShadowUncapturedLocal : DiagGroup<"shadow-uncaptured-local">; // -Wshadow-all is a catch-all for all shadowing. -Wshadow is just the // shadowing that we think is unsafe. def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified, ShadowIvar]>; -def ShadowAll : DiagGroup<"shadow-all", [Shadow, ShadowFieldInConstructor]>; +def ShadowAll : DiagGroup<"shadow-all", [Shadow, ShadowFieldInConstructor, + ShadowUncapturedLocal]>; def Shorten64To32 : DiagGroup<"shorten-64-to-32">; def : DiagGroup<"sign-promo">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 524a9b9..0ff1a6f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -357,6 +357,9 @@ def warn_decl_shadow : "static data member of %2|" "field of %2}1">, InGroup, DefaultIgnore; +def warn_decl_shadow_uncaptured_local : + Warning, + InGroup, DefaultIgnore; def warn_ctor_parm_shadows_field: Warning<"constructor parameter %0 shadows the field %1 of %2">, InGroup, DefaultIgnore; @@ -6230,6 +6233,8 @@ let CategoryName = "Lambda Issue" in { def note_lambda_to_block_conv : Note< "implicit capture of lambda object due to conversion to block pointer " "here">; + def note_var_explicitly_captured_here : Note<"variable %0 is explicitly " + "captured here">; // C++14 lambda init-captures. def warn_cxx11_compat_init_capture : Warning< diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index e20ae1a..1859b7f 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -6582,6 +6582,17 @@ static ShadowedDeclKind computeShadowedDeclKind(const NamedDecl *ShadowedDecl, return OldDC->isFileContext() ? SDK_Global : SDK_Local; } +/// 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) { + for (const LambdaScopeInfo::Capture &Capture : LSI->Captures) { + if (Capture.isVariableCapture() && Capture.getVariable() == VD) + return Capture.getLocation(); + } + return SourceLocation(); +} + /// \brief Diagnose variable or built-in function shadowing. Implements /// -Wshadow. /// @@ -6640,6 +6651,22 @@ void Sema::CheckShadow(Scope *S, VarDecl *D, const LookupResult& R) { DeclContext *OldDC = ShadowedDecl->getDeclContext(); + unsigned WarningDiag = diag::warn_decl_shadow; + SourceLocation CaptureLoc; + if (isa(ShadowedDecl) && NewDC && isa(NewDC)) { + if (const auto *RD = dyn_cast(NewDC->getParent())) { + // Try to avoid warnings for lambdas with an explicit capture list. + if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent()) && + RD->getLambdaCaptureDefault() == LCD_None) { + 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; + } + } + } + // Only warn about certain kinds of shadowing for class members. if (NewDC && NewDC->isRecord()) { // In particular, don't warn about shadowing non-class members. @@ -6661,7 +6688,9 @@ void Sema::CheckShadow(Scope *S, VarDecl *D, const LookupResult& R) { if (getSourceManager().isInSystemMacro(R.getNameLoc())) return; ShadowedDeclKind Kind = computeShadowedDeclKind(ShadowedDecl, OldDC); - Diag(R.getNameLoc(), diag::warn_decl_shadow) << Name << Kind << OldDC; + Diag(R.getNameLoc(), WarningDiag) << Name << Kind << OldDC; + if (!CaptureLoc.isInvalid()) + Diag(CaptureLoc, diag::note_var_explicitly_captured_here) << Name; 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 new file mode 100644 index 0000000..24f1a34 --- /dev/null +++ b/clang/test/SemaCXX/warn-shadow-in-lambdas.cpp @@ -0,0 +1,91 @@ +// 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 + +void foo(int param) { // expected-note 1+ {{previous declaration is here}} + int var = 0; // expected-note 1+ {{previous declaration is here}} + + // Warn for lambdas with a default capture specifier. + { + auto f1 = [=] { int var = 1; }; // expected-warning {{declaration shadows a local variable}} + auto f2 = [&] { int var = 2; }; // expected-warning {{declaration shadows a local variable}} + auto f3 = [=] (int param) { ; }; // expected-warning {{declaration shadows a local variable}} + auto f4 = [&] (int param) { ; }; // expected-warning {{declaration shadows a local variable}} + } + + // Warn normally inside of lambdas. + auto l1 = [] { // expected-note {{previous declaration is here}} + int x = 1; // expected-note {{previous declaration is here}} + { int x = 2; } // expected-warning {{declaration shadows a local variable}} + }; + auto l2 = [] (int x) { // expected-note {{previous declaration is here}} + { int x = 1; } // expected-warning {{declaration shadows a local variable}} + }; + + // Avoid warnings for variables that aren't explicitly captured. + { +#ifdef AVOID + auto f1 = [] { int var = 1; }; // no warning + auto f2 = [] (int param) { ; }; // no warning + auto f3 = [param] () { int var = 1; }; // no warning + auto f4 = [var] (int param) { ; }; // no warning +#else + auto f1 = [] { int var = 1; }; // expected-warning {{declaration shadows a local variable}} + auto f2 = [] (int param) { ; }; // expected-warning {{declaration shadows a local variable}} + auto f3 = [param] () { int var = 1; }; // expected-warning {{declaration shadows a local variable}} + auto f4 = [var] (int param) { ; }; // expected-warning {{declaration shadows a local variable}} +#endif + }; + + // Warn for variables that are explicitly captured. + { + auto f1 = [var] () { // expected-note {{variable 'var' is explicitly captured here}} + int var = 1; // expected-warning {{declaration shadows a local variable}} + }; + auto f2 = [param] // expected-note {{variable 'param' is explicitly captured here}} + (int param) { ; }; // expected-warning {{declaration shadows a local variable}} + } + + // Warn for variables defined in the capture list. + auto l3 = [z = var] { // expected-note {{previous declaration is here}} +#ifdef AVOID + int var = 1; // no warning +#else + int var = 1; // expected-warning {{declaration shadows a local variable}} +#endif + { int z = 1; } // expected-warning {{declaration shadows a local variable}} + }; +#ifdef AVOID + auto l4 = [var = param] (int param) { ; }; // no warning +#else + auto l4 = [var = param] (int param) { ; }; // expected-warning {{declaration shadows a local variable}} +#endif + + // Make sure that inner lambdas work as well. + auto l5 = [var, l1] { // expected-note {{variable 'l1' is explicitly captured here}} + auto l1 = [] { // expected-warning {{declaration shadows a local variable}} +#ifdef AVOID + int var = 1; // no warning +#else + int var = 1; // expected-warning {{declaration shadows a local variable}} +#endif + }; +#ifdef AVOID + auto f1 = [] { int var = 1; }; // no warning +#else + auto f1 = [] { int var = 1; }; // expected-warning {{declaration shadows a local variable}} +#endif + auto f2 = [=] { int var = 1; }; // expected-warning {{declaration shadows a local variable}} + auto f3 = [var] // expected-note {{variable 'var' is explicitly captured here}} + { int var = 1; }; // expected-warning {{declaration shadows a local variable}} + }; + + // Generic lambda arguments should work. +#ifdef AVOID + auto g1 = [](auto param) { ; }; // no warning +#else + auto g1 = [](auto param) { ; }; // expected-warning {{declaration shadows a local variable}} +#endif + auto g2 = [param] // expected-note {{variable 'param' is explicitly captured here}} + (auto param) { ; }; // expected-warning {{declaration shadows a local variable}} +} -- 2.7.4