Skip to content

Commit 348c144

Browse files
committedApr 3, 2017
Enhance -Wshadow to warn when shadowing typedefs or type aliases
Enhance -Wshadow to emit a warning when typedefs or type aliases are shadowed. Fixes bug https://bugs.llvm.org//show_bug.cgi?id=28676. Patch by Ahmed Asadi. Differential Revision: https://reviews.llvm.org/D31235 llvm-svn: 299363
1 parent d33ee1b commit 348c144

File tree

4 files changed

+155
-24
lines changed

4 files changed

+155
-24
lines changed
 

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

+3-1
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,9 @@ def warn_decl_shadow :
369369
"local variable|"
370370
"variable in %2|"
371371
"static data member of %2|"
372-
"field of %2}1">,
372+
"field of %2|"
373+
"typedef in %2|"
374+
"type alias in %2}1">,
373375
InGroup<Shadow>, DefaultIgnore;
374376
def warn_decl_shadow_uncaptured_local :
375377
Warning<warn_decl_shadow.Text>,

‎clang/include/clang/Sema/Sema.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -1738,8 +1738,11 @@ class Sema {
17381738

17391739
static bool adjustContextForLocalExternDecl(DeclContext *&DC);
17401740
void DiagnoseFunctionSpecifiers(const DeclSpec &DS);
1741+
NamedDecl *getShadowedDeclaration(const TypedefNameDecl *D,
1742+
const LookupResult &R);
17411743
NamedDecl *getShadowedDeclaration(const VarDecl *D, const LookupResult &R);
1742-
void CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl, const LookupResult &R);
1744+
void CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
1745+
const LookupResult &R);
17431746
void CheckShadow(Scope *S, VarDecl *D);
17441747

17451748
/// Warn if 'E', which is an expression that is about to be modified, refers

‎clang/lib/Sema/SemaDecl.cpp

+55-18
Original file line numberDiff line numberDiff line change
@@ -5530,6 +5530,10 @@ Sema::CheckTypedefForVariablyModifiedType(Scope *S, TypedefNameDecl *NewTD) {
55305530
NamedDecl*
55315531
Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD,
55325532
LookupResult &Previous, bool &Redeclaration) {
5533+
5534+
// Find the shadowed declaration before filtering for scope.
5535+
NamedDecl *ShadowedDecl = getShadowedDeclaration(NewTD, Previous);
5536+
55335537
// Merge the decl with the existing one if appropriate. If the decl is
55345538
// in an outer scope, it isn't the same thing.
55355539
FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage*/false,
@@ -5540,6 +5544,9 @@ Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD,
55405544
MergeTypedefNameDecl(S, NewTD, Previous);
55415545
}
55425546

5547+
if (ShadowedDecl && !Redeclaration)
5548+
CheckShadow(NewTD, ShadowedDecl, Previous);
5549+
55435550
// If this is the C FILE type, notify the AST context.
55445551
if (IdentifierInfo *II = NewTD->getIdentifier())
55455552
if (!NewTD->isInvalidDecl() &&
@@ -6671,13 +6678,25 @@ NamedDecl *Sema::ActOnVariableDeclarator(
66716678
}
66726679

66736680
/// Enum describing the %select options in diag::warn_decl_shadow.
6674-
enum ShadowedDeclKind { SDK_Local, SDK_Global, SDK_StaticMember, SDK_Field };
6681+
enum ShadowedDeclKind {
6682+
SDK_Local,
6683+
SDK_Global,
6684+
SDK_StaticMember,
6685+
SDK_Field,
6686+
SDK_Typedef,
6687+
SDK_Using
6688+
};
66756689

66766690
/// Determine what kind of declaration we're shadowing.
66776691
static ShadowedDeclKind computeShadowedDeclKind(const NamedDecl *ShadowedDecl,
66786692
const DeclContext *OldDC) {
6679-
if (isa<RecordDecl>(OldDC))
6693+
if (isa<TypeAliasDecl>(ShadowedDecl))
6694+
return SDK_Using;
6695+
else if (isa<TypedefDecl>(ShadowedDecl))
6696+
return SDK_Typedef;
6697+
else if (isa<RecordDecl>(OldDC))
66806698
return isa<FieldDecl>(ShadowedDecl) ? SDK_Field : SDK_StaticMember;
6699+
66816700
return OldDC->isFileContext() ? SDK_Global : SDK_Local;
66826701
}
66836702

@@ -6692,28 +6711,44 @@ static SourceLocation getCaptureLocation(const LambdaScopeInfo *LSI,
66926711
return SourceLocation();
66936712
}
66946713

6714+
static bool shouldWarnIfShadowedDecl(const DiagnosticsEngine &Diags,
6715+
const LookupResult &R) {
6716+
// Only diagnose if we're shadowing an unambiguous field or variable.
6717+
if (R.getResultKind() != LookupResult::Found)
6718+
return false;
6719+
6720+
// Return false if warning is ignored.
6721+
return !Diags.isIgnored(diag::warn_decl_shadow, R.getNameLoc());
6722+
}
6723+
66956724
/// \brief Return the declaration shadowed by the given variable \p D, or null
66966725
/// if it doesn't shadow any declaration or shadowing warnings are disabled.
66976726
NamedDecl *Sema::getShadowedDeclaration(const VarDecl *D,
66986727
const LookupResult &R) {
6699-
// Return if warning is ignored.
6700-
if (Diags.isIgnored(diag::warn_decl_shadow, R.getNameLoc()))
6728+
if (!shouldWarnIfShadowedDecl(Diags, R))
67016729
return nullptr;
67026730

67036731
// Don't diagnose declarations at file scope.
67046732
if (D->hasGlobalStorage())
67056733
return nullptr;
67066734

6707-
// Only diagnose if we're shadowing an unambiguous field or variable.
6708-
if (R.getResultKind() != LookupResult::Found)
6709-
return nullptr;
6710-
67116735
NamedDecl *ShadowedDecl = R.getFoundDecl();
67126736
return isa<VarDecl>(ShadowedDecl) || isa<FieldDecl>(ShadowedDecl)
67136737
? ShadowedDecl
67146738
: nullptr;
67156739
}
67166740

6741+
/// \brief Return the declaration shadowed by the given typedef \p D, or null
6742+
/// if it doesn't shadow any declaration or shadowing warnings are disabled.
6743+
NamedDecl *Sema::getShadowedDeclaration(const TypedefNameDecl *D,
6744+
const LookupResult &R) {
6745+
if (!shouldWarnIfShadowedDecl(Diags, R))
6746+
return nullptr;
6747+
6748+
NamedDecl *ShadowedDecl = R.getFoundDecl();
6749+
return isa<TypedefNameDecl>(ShadowedDecl) ? ShadowedDecl : nullptr;
6750+
}
6751+
67176752
/// \brief Diagnose variable or built-in function shadowing. Implements
67186753
/// -Wshadow.
67196754
///
@@ -6723,7 +6758,7 @@ NamedDecl *Sema::getShadowedDeclaration(const VarDecl *D,
67236758
/// \param ShadowedDecl the declaration that is shadowed by the given variable
67246759
/// \param R the lookup of the name
67256760
///
6726-
void Sema::CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl,
6761+
void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
67276762
const LookupResult &R) {
67286763
DeclContext *NewDC = D->getDeclContext();
67296764

@@ -6735,13 +6770,13 @@ void Sema::CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl,
67356770

67366771
// Fields shadowed by constructor parameters are a special case. Usually
67376772
// the constructor initializes the field with the parameter.
6738-
if (isa<CXXConstructorDecl>(NewDC) && isa<ParmVarDecl>(D)) {
6739-
// Remember that this was shadowed so we can either warn about its
6740-
// modification or its existence depending on warning settings.
6741-
D = D->getCanonicalDecl();
6742-
ShadowingDecls.insert({D, FD});
6743-
return;
6744-
}
6773+
if (isa<CXXConstructorDecl>(NewDC))
6774+
if (const auto PVD = dyn_cast<ParmVarDecl>(D)) {
6775+
// Remember that this was shadowed so we can either warn about its
6776+
// modification or its existence depending on warning settings.
6777+
ShadowingDecls.insert({PVD->getCanonicalDecl(), FD});
6778+
return;
6779+
}
67456780
}
67466781

67476782
if (VarDecl *shadowedVar = dyn_cast<VarDecl>(ShadowedDecl))
@@ -6759,7 +6794,8 @@ void Sema::CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl,
67596794

67606795
unsigned WarningDiag = diag::warn_decl_shadow;
67616796
SourceLocation CaptureLoc;
6762-
if (isa<VarDecl>(ShadowedDecl) && NewDC && isa<CXXMethodDecl>(NewDC)) {
6797+
if (isa<VarDecl>(D) && isa<VarDecl>(ShadowedDecl) && NewDC &&
6798+
isa<CXXMethodDecl>(NewDC)) {
67636799
if (const auto *RD = dyn_cast<CXXRecordDecl>(NewDC->getParent())) {
67646800
if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) {
67656801
if (RD->getLambdaCaptureDefault() == LCD_None) {
@@ -6773,7 +6809,8 @@ void Sema::CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl,
67736809
// Remember that this was shadowed so we can avoid the warning if the
67746810
// shadowed decl isn't captured and the warning settings allow it.
67756811
cast<LambdaScopeInfo>(getCurFunction())
6776-
->ShadowingDecls.push_back({D, cast<VarDecl>(ShadowedDecl)});
6812+
->ShadowingDecls.push_back(
6813+
{cast<VarDecl>(D), cast<VarDecl>(ShadowedDecl)});
67776814
return;
67786815
}
67796816
}

‎clang/test/SemaCXX/warn-shadow.cpp

+93-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow-all %s
1+
// RUN: %clang_cc1 -verify -fsyntax-only -std=c++11 -Wshadow-all %s
22

33
namespace {
44
int i; // expected-note {{previous declaration is here}}
@@ -7,14 +7,21 @@ namespace {
77
namespace one {
88
namespace two {
99
int j; // expected-note {{previous declaration is here}}
10+
typedef int jj; // expected-note 2 {{previous declaration is here}}
11+
using jjj=int; // expected-note 2 {{previous declaration is here}}
1012
}
1113
}
1214

1315
namespace xx {
1416
int m;
17+
typedef int mm;
18+
using mmm=int;
19+
1520
}
1621
namespace yy {
1722
int m;
23+
typedef char mm;
24+
using mmm=char;
1825
}
1926

2027
using namespace one::two;
@@ -25,14 +32,19 @@ void foo() {
2532
int i; // expected-warning {{declaration shadows a variable in namespace '(anonymous)'}}
2633
int j; // expected-warning {{declaration shadows a variable in namespace 'one::two'}}
2734
int m;
35+
int mm;
36+
int mmm;
2837
}
2938

3039
class A {
31-
static int data; // expected-note {{previous declaration}}
32-
// expected-note@+1 {{previous declaration}}
40+
static int data; // expected-note 1 {{previous declaration}}
41+
// expected-note@+1 1 {{previous declaration}}
3342
int field;
3443
int f1, f2, f3, f4; // expected-note 8 {{previous declaration is here}}
3544

45+
typedef int a1; // expected-note 2 {{previous declaration}}
46+
using a2=int; // expected-note 2 {{previous declaration}}
47+
3648
// The initialization is safe, but the modifications are not.
3749
A(int f1, int f2, int f3, int f4) // expected-note-re 4 {{variable 'f{{[0-4]}}' is declared here}}
3850
: f1(f1) {
@@ -50,6 +62,28 @@ class A {
5062
void test() {
5163
char *field; // expected-warning {{declaration shadows a field of 'A'}}
5264
char *data; // expected-warning {{declaration shadows a static data member of 'A'}}
65+
char *a1; // no warning
66+
char *a2; // no warning
67+
char *jj; // no warning
68+
char *jjj; // no warning
69+
}
70+
71+
void test2() {
72+
typedef char field; // no warning
73+
typedef char data; // no warning
74+
typedef char a1; // expected-warning {{declaration shadows a typedef in 'A'}}
75+
typedef char a2; // expected-warning {{declaration shadows a type alias in 'A'}}
76+
typedef char jj; // expected-warning {{declaration shadows a typedef in namespace 'one::two'}}
77+
typedef char jjj; // expected-warning {{declaration shadows a type alias in namespace 'one::two'}}
78+
}
79+
80+
void test3() {
81+
using field=char; // no warning
82+
using data=char; // no warning
83+
using a1=char; // expected-warning {{declaration shadows a typedef in 'A'}}
84+
using a2=char; // expected-warning {{declaration shadows a type alias in 'A'}}
85+
using jj=char; // expected-warning {{declaration shadows a typedef in namespace 'one::two'}}
86+
using jjj=char; // expected-warning {{declaration shadows a type alias in namespace 'one::two'}}
5387
}
5488
};
5589

@@ -63,13 +97,23 @@ class B : A {
6397
namespace rdar8900456 {
6498
struct Foo {
6599
static void Baz();
100+
static void Baz1();
101+
static void Baz2();
66102
private:
67103
int Bar;
68104
};
69105

70106
void Foo::Baz() {
71107
double Bar = 12; // Don't warn.
72108
}
109+
110+
void Foo::Baz1() {
111+
typedef int Bar; // Don't warn.
112+
}
113+
114+
void Foo::Baz2() {
115+
using Bar=int; // Don't warn.
116+
}
73117
}
74118

75119
// http://llvm.org/PR9160
@@ -87,7 +131,9 @@ struct S {
87131
};
88132
}
89133

90-
extern int bob; // expected-note {{previous declaration is here}}
134+
extern int bob; // expected-note 1 {{previous declaration is here}}
135+
typedef int bob1; // expected-note 2 {{previous declaration is here}}
136+
using bob2=int; // expected-note 2 {{previous declaration is here}}
91137

92138
// rdar://8883302
93139
void rdar8883302() {
@@ -96,6 +142,20 @@ void rdar8883302() {
96142

97143
void test8() {
98144
int bob; // expected-warning {{declaration shadows a variable in the global namespace}}
145+
int bob1; //no warning
146+
int bob2; // no warning
147+
}
148+
149+
void test9() {
150+
typedef int bob; // no warning
151+
typedef int bob1; // expected-warning {{declaration shadows a typedef in the global namespace}}
152+
typedef int bob2; // expected-warning {{declaration shadows a type alias in the global namespace}}
153+
}
154+
155+
void test10() {
156+
using bob=int; // no warning
157+
using bob1=int; // expected-warning {{declaration shadows a typedef in the global namespace}}
158+
using bob2=int; // expected-warning {{declaration shadows a type alias in the global namespace}}
99159
}
100160

101161
namespace rdar29067894 {
@@ -104,6 +164,35 @@ void avoidWarningWhenRedefining(int b) { // expected-note {{previous definition
104164
int a = 0; // expected-note {{previous definition is here}}
105165
int a = 1; // expected-error {{redefinition of 'a'}}
106166
int b = 2; // expected-error {{redefinition of 'b'}}
167+
168+
using c=char; // expected-note {{previous definition is here}}
169+
using c=int; // expected-error {{type alias redefinition with different types ('int' vs 'char')}}
170+
171+
typedef char d; // expected-note {{previous definition is here}}
172+
typedef int d; // expected-error {{typedef redefinition with different types ('int' vs 'char')}}
173+
174+
using e=char; // expected-note {{previous definition is here}}
175+
typedef int e; // expected-error {{type alias redefinition with different types ('int' vs 'char')}}
176+
177+
int f; // expected-note {{previous definition is here}}
178+
using f=int; // expected-error {{redefinition of 'f'}}
179+
180+
using g=int; // expected-note {{previous definition is here}}
181+
int g; // expected-error {{redefinition of 'g'}}
182+
183+
typedef int h; // expected-note {{previous definition is here}}
184+
int h; // expected-error {{redefinition of 'h'}}
185+
186+
int k; // expected-note {{previous definition is here}}
187+
typedef int k; // expected-error {{redefinition of 'k'}}
188+
189+
using l=char; // no warning or error.
190+
using l=char; // no warning or error.
191+
typedef char l; // no warning or error.
192+
193+
typedef char n; // no warning or error.
194+
typedef char n; // no warning or error.
195+
using n=char; // no warning or error.
107196
}
108197

109198
}

0 commit comments

Comments
 (0)
Please sign in to comment.