Skip to content

Commit

Permalink
[Sema] Avoid -Wshadow warning when a "redefinition of " error is pres…
Browse files Browse the repository at this point in the history
…ented

This commit ensures that clang avoids the redundant -Wshadow warning for
variables that already get a "redefinition of " error.

rdar://29067894

Differential Revision: https://reviews.llvm.org/D28350

llvm-svn: 291564
  • Loading branch information
hyp committed Jan 10, 2017
1 parent 00b3f3c commit 0e61372
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 23 deletions.
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
Expand Up @@ -1709,7 +1709,8 @@ class Sema {

static bool adjustContextForLocalExternDecl(DeclContext *&DC);
void DiagnoseFunctionSpecifiers(const DeclSpec &DS);
void CheckShadow(Scope *S, VarDecl *D, const LookupResult& R);
NamedDecl *getShadowedDeclaration(const VarDecl *D, const LookupResult &R);
void CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl, const LookupResult &R);
void CheckShadow(Scope *S, VarDecl *D);

/// Warn if 'E', which is an expression that is about to be modified, refers
Expand Down
57 changes: 35 additions & 22 deletions clang/lib/Sema/SemaDecl.cpp
Expand Up @@ -6426,9 +6426,10 @@ NamedDecl *Sema::ActOnVariableDeclarator(
}
}

// Diagnose shadowed variables before filtering for scope.
if (D.getCXXScopeSpec().isEmpty())
CheckShadow(S, NewVD, Previous);
// Find the shadowed declaration before filtering for scope.
NamedDecl *ShadowedDecl = D.getCXXScopeSpec().isEmpty()
? getShadowedDeclaration(NewVD, Previous)
: nullptr;

// Don't consider existing declarations that are in a different
// scope and are out-of-semantic-context declarations (if the new
Expand Down Expand Up @@ -6523,6 +6524,10 @@ NamedDecl *Sema::ActOnVariableDeclarator(
}
}

// Diagnose shadowed variables iff this isn't a redeclaration.
if (ShadowedDecl && !D.isRedeclaration())
CheckShadow(NewVD, ShadowedDecl, Previous);

ProcessPragmaWeak(S, NewVD);

// If this is the first declaration of an extern C variable, update
Expand Down Expand Up @@ -6596,33 +6601,40 @@ static SourceLocation getCaptureLocation(const LambdaScopeInfo *LSI,
return SourceLocation();
}

/// \brief Diagnose variable or built-in function shadowing. Implements
/// -Wshadow.
///
/// This method is called whenever a VarDecl is added to a "useful"
/// scope.
///
/// \param S the scope in which the shadowing name is being declared
/// \param R the lookup of the name
///
void Sema::CheckShadow(Scope *S, VarDecl *D, const LookupResult& R) {
/// \brief Return the declaration shadowed by the given variable \p D, or null
/// if it doesn't shadow any declaration or shadowing warnings are disabled.
NamedDecl *Sema::getShadowedDeclaration(const VarDecl *D,
const LookupResult &R) {
// Return if warning is ignored.
if (Diags.isIgnored(diag::warn_decl_shadow, R.getNameLoc()))
return;
return nullptr;

// Don't diagnose declarations at file scope.
if (D->hasGlobalStorage())
return;

DeclContext *NewDC = D->getDeclContext();
return nullptr;

// Only diagnose if we're shadowing an unambiguous field or variable.
if (R.getResultKind() != LookupResult::Found)
return;
return nullptr;

NamedDecl* ShadowedDecl = R.getFoundDecl();
if (!isa<VarDecl>(ShadowedDecl) && !isa<FieldDecl>(ShadowedDecl))
return;
NamedDecl *ShadowedDecl = R.getFoundDecl();
return isa<VarDecl>(ShadowedDecl) || isa<FieldDecl>(ShadowedDecl)
? ShadowedDecl
: nullptr;
}

/// \brief Diagnose variable or built-in function shadowing. Implements
/// -Wshadow.
///
/// This method is called whenever a VarDecl is added to a "useful"
/// scope.
///
/// \param ShadowedDecl the declaration that is shadowed by the given variable
/// \param R the lookup of the name
///
void Sema::CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl,
const LookupResult &R) {
DeclContext *NewDC = D->getDeclContext();

if (FieldDecl *FD = dyn_cast<FieldDecl>(ShadowedDecl)) {
// Fields are not shadowed by variables in C++ static methods.
Expand Down Expand Up @@ -6733,7 +6745,8 @@ void Sema::CheckShadow(Scope *S, VarDecl *D) {
LookupResult R(*this, D->getDeclName(), D->getLocation(),
Sema::LookupOrdinaryName, Sema::ForRedeclaration);
LookupName(R, S);
CheckShadow(S, D, R);
if (NamedDecl *ShadowedDecl = getShadowedDeclaration(D, R))
CheckShadow(D, ShadowedDecl, R);
}

/// Check if 'E', which is an expression that is about to be modified, refers
Expand Down
8 changes: 8 additions & 0 deletions clang/test/SemaCXX/warn-shadow-in-lambdas.cpp
Expand Up @@ -137,3 +137,11 @@ void foo(int param) { // expected-note 1+ {{previous declaration is here}}
auto g3 = [param] // expected-note {{variable 'param' is explicitly captured here}}
(auto param) { ; }; // expected-warning {{declaration shadows a local variable}}
}

void avoidWarningWhenRedefining() {
int a = 1;
auto l = [b = a] { // expected-note {{previous definition is here}}
// Don't warn on redefinitions.
int b = 0; // expected-error {{redefinition of 'b'}}
};
}
10 changes: 10 additions & 0 deletions clang/test/SemaCXX/warn-shadow.cpp
Expand Up @@ -97,3 +97,13 @@ void rdar8883302() {
void test8() {
int bob; // expected-warning {{declaration shadows a variable in the global namespace}}
}

namespace rdar29067894 {

void avoidWarningWhenRedefining(int b) { // expected-note {{previous definition is here}}
int a = 0; // expected-note {{previous definition is here}}
int a = 1; // expected-error {{redefinition of 'a'}}
int b = 2; // expected-error {{redefinition of 'b'}}
}

}

0 comments on commit 0e61372

Please sign in to comment.