<div dir="ltr">Nice!</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 7, 2017 at 7:30 PM, Saleem Abdulrasool via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: compnerd<br>
Date: Tue Feb 7 21:30:13 2017<br>
New Revision: 294401<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=294401&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=294401&view=rev</a><br>
Log:<br>
Sema: add warning for c++ member variable shadowing<br>
<br>
Add a warning for shadowed variables across records. Referencing a<br>
shadow'ed variable may not give the desired variable. Add an optional<br>
warning for the shadowing.<br>
<br>
Patch by James Sun!<br>
<br>
Added:<br>
cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p10.cpp<br>
Modified:<br>
cfe/trunk/include/clang/Basic/<wbr>DiagnosticGroups.td<br>
cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td<br>
cfe/trunk/include/clang/Sema/<wbr>Sema.h<br>
cfe/trunk/lib/Sema/<wbr>SemaDeclCXX.cpp<br>
cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p6.cpp<br>
cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p7.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/<wbr>DiagnosticGroups.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=294401&r1=294400&r2=294401&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Basic/DiagnosticGroups.<wbr>td?rev=294401&r1=294400&r2=<wbr>294401&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Basic/<wbr>DiagnosticGroups.td (original)<br>
+++ cfe/trunk/include/clang/Basic/<wbr>DiagnosticGroups.td Tue Feb 7 21:30:13 2017<br>
@@ -355,6 +355,7 @@ def SemiBeforeMethodBody : DiagGroup<"se<br>
def Sentinel : DiagGroup<"sentinel">;<br>
def MissingMethodReturnType : DiagGroup<"missing-method-<wbr>return-type">;<br>
<br>
+def ShadowField : DiagGroup<"shadow-field">;<br>
def ShadowFieldInConstructorModifi<wbr>ed : DiagGroup<"shadow-field-in-<wbr>constructor-modified">;<br>
def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-<wbr>constructor",<br>
[<wbr>ShadowFieldInConstructorModifi<wbr>ed]>;<br>
@@ -366,7 +367,7 @@ def ShadowUncapturedLocal : DiagGroup<"s<br>
def Shadow : DiagGroup<"shadow", [<wbr>ShadowFieldInConstructorModifi<wbr>ed,<br>
ShadowIvar]>;<br>
def ShadowAll : DiagGroup<"shadow-all", [Shadow, ShadowFieldInConstructor,<br>
- ShadowUncapturedLocal]>;<br>
+ ShadowUncapturedLocal, ShadowField]>;<br>
<br>
def Shorten64To32 : DiagGroup<"shorten-64-to-32">;<br>
def : DiagGroup<"sign-promo">;<br>
<br>
Modified: cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=294401&r1=294400&r2=294401&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Basic/<wbr>DiagnosticSemaKinds.td?rev=<wbr>294401&r1=294400&r2=294401&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td Tue Feb 7 21:30:13 2017<br>
@@ -8959,4 +8959,9 @@ def ext_warn_gnu_final : ExtWarn<<br>
"__final is a GNU extension, consider using C++11 final">,<br>
InGroup<GccCompat>;<br>
<br>
+def warn_shadow_field :<br>
+ Warning<"non-static data member '%0' of '%1' shadows member inherited from type '%2'">,<br>
+ InGroup<ShadowField>, DefaultIgnore;<br>
+def note_shadow_field : Note<"declared here">;<br>
+<br>
} // end of sema component.<br>
<br>
Modified: cfe/trunk/include/clang/Sema/<wbr>Sema.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=294401&r1=294400&r2=294401&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Sema/Sema.h?rev=294401&<wbr>r1=294400&r2=294401&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Sema/<wbr>Sema.h (original)<br>
+++ cfe/trunk/include/clang/Sema/<wbr>Sema.h Tue Feb 7 21:30:13 2017<br>
@@ -10074,6 +10074,11 @@ private:<br>
void CheckBitFieldInitialization(<wbr>SourceLocation InitLoc, FieldDecl *Field,<br>
Expr *Init);<br>
<br>
+ /// Check if there is a field shadowing.<br>
+ void CheckShadowInheritedFields(<wbr>const SourceLocation &Loc,<br>
+ DeclarationName FieldName,<br>
+ const CXXRecordDecl *RD);<br>
+<br>
/// \brief Check if the given expression contains 'break' or 'continue'<br>
/// statement that produces control flow different from GCC.<br>
void CheckBreakContinueBinding(Expr *E);<br>
<br>
Modified: cfe/trunk/lib/Sema/<wbr>SemaDeclCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=294401&r1=294400&r2=294401&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>SemaDeclCXX.cpp?rev=294401&r1=<wbr>294400&r2=294401&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/<wbr>SemaDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/<wbr>SemaDeclCXX.cpp Tue Feb 7 21:30:13 2017<br>
@@ -2758,6 +2758,56 @@ static AttributeList *getMSPropertyAttr(<br>
return nullptr;<br>
}<br>
<br>
+// Check if there is a field shadowing.<br>
+void Sema::<wbr>CheckShadowInheritedFields(<wbr>const SourceLocation &Loc,<br>
+ DeclarationName FieldName,<br>
+ const CXXRecordDecl *RD) {<br>
+ if (Diags.isIgnored(diag::warn_<wbr>shadow_field, Loc))<br>
+ return;<br>
+<br>
+ // To record a shadowed field in a base<br>
+ std::map<CXXRecordDecl*, NamedDecl*> Bases;<br>
+ auto FieldShadowed = [&](const CXXBaseSpecifier *Specifier,<br>
+ CXXBasePath &Path) {<br>
+ const auto Base = Specifier->getType()-><wbr>getAsCXXRecordDecl();<br>
+ // Record an ambiguous path directly<br>
+ if (Bases.find(Base) != Bases.end())<br>
+ return true;<br>
+ for (const auto Field : Base->lookup(FieldName)) {<br>
+ if ((isa<FieldDecl>(Field) || isa<IndirectFieldDecl>(Field)) &&<br>
+ Field->getAccess() != AS_private) {<br>
+ assert(Field->getAccess() != AS_none);<br>
+ assert(Bases.find(Base) == Bases.end());<br>
+ Bases[Base] = Field;<br>
+ return true;<br>
+ }<br>
+ }<br>
+ return false;<br>
+ };<br>
+<br>
+ CXXBasePaths Paths(/*FindAmbiguities=*/<wbr>true, /*RecordPaths=*/true,<br>
+ /*DetectVirtual=*/true);<br>
+ if (!RD->lookupInBases(<wbr>FieldShadowed, Paths))<br>
+ return;<br>
+<br>
+ for (const auto &P : Paths) {<br>
+ auto Base = P.back().Base->getType()-><wbr>getAsCXXRecordDecl();<br>
+ auto It = Bases.find(Base);<br>
+ // Skip duplicated bases<br>
+ if (It == Bases.end())<br>
+ continue;<br>
+ auto BaseField = It->second;<br>
+ assert(BaseField->getAccess() != AS_private);<br>
+ if (AS_none !=<br>
+ CXXRecordDecl::MergeAccess(P.<wbr>Access, BaseField->getAccess())) {<br>
+ Diag(Loc, diag::warn_shadow_field)<br>
+ << FieldName.getAsString() << RD->getName() << Base->getName();<br>
+ Diag(BaseField->getLocation(), diag::note_shadow_field);<br>
+ Bases.erase(It);<br>
+ }<br>
+ }<br>
+}<br>
+<br>
/// ActOnCXXMemberDeclarator - This is invoked when a C++ class member<br>
/// declarator is parsed. 'AS' is the access specifier, 'BW' specifies the<br>
/// bitfield width if there is one, 'InitExpr' specifies the initializer if<br>
@@ -2957,6 +3007,11 @@ Sema::<wbr>ActOnCXXMemberDeclarator(Scope *S,<br>
if (!Member)<br>
return nullptr;<br>
}<br>
+<br>
+ // Check for any possible shadowed member variables<br>
+ if (const auto *RD = cast<CXXRecordDecl>(<wbr>CurContext))<br>
+ CheckShadowInheritedFields(<wbr>Loc, Name, RD);<br>
+<br>
} else {<br>
Member = HandleDeclarator(S, D, TemplateParameterLists);<br>
if (!Member)<br>
<br>
Added: cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p10.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p10.cpp?rev=294401&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/CXX/<wbr>class.derived/class.member.<wbr>lookup/p10.cpp?rev=294401&<wbr>view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p10.cpp (added)<br>
+++ cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p10.cpp Tue Feb 7 21:30:13 2017<br>
@@ -0,0 +1,114 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wshadow-all<br>
+<br>
+// Basic cases, ambiguous paths, and fields with different access<br>
+class A {<br>
+public:<br>
+ int x; // expected-note 2{{declared here}}<br>
+protected:<br>
+ int y; // expected-note 2{{declared here}}<br>
+private:<br>
+ int z;<br>
+};<br>
+<br>
+struct B : A {<br>
+};<br>
+<br>
+struct C : A {<br>
+};<br>
+<br>
+struct W {<br>
+ int w; // expected-note {{declared here}}<br>
+};<br>
+<br>
+struct U : W {<br>
+};<br>
+<br>
+struct V : W {<br>
+};<br>
+<br>
+class D {<br>
+public:<br>
+ char w; // expected-note {{declared here}}<br>
+private:<br>
+ char x;<br>
+};<br>
+<br>
+// Check direct inheritance and multiple paths to the same base.<br>
+class E : B, C, D, U, V<br>
+{<br>
+ unsigned x; // expected-warning {{non-static data member 'x' of 'E' shadows member inherited from type 'A'}}<br>
+ char y; // expected-warning {{non-static data member 'y' of 'E' shadows member inherited from type 'A'}}<br>
+ double z;<br>
+ char w; // expected-warning {{non-static data member 'w' of 'E' shadows member inherited from type 'D'}} expected-warning {{non-static data member 'w' of 'E' shadows member inherited from type 'W'}}<br>
+};<br>
+<br>
+// Virtual inheritance<br>
+struct F : virtual A {<br>
+};<br>
+<br>
+struct G : virtual A {<br>
+};<br>
+<br>
+class H : F, G {<br>
+ int x; // expected-warning {{non-static data member 'x' of 'H' shadows member inherited from type 'A'}}<br>
+ int y; // expected-warning {{non-static data member 'y' of 'H' shadows member inherited from type 'A'}}<br>
+ int z;<br>
+};<br>
+<br>
+// Indirect inheritance<br>
+struct I {<br>
+ union {<br>
+ int x; // expected-note {{declared here}}<br>
+ int y;<br>
+ };<br>
+};<br>
+<br>
+struct J : I {<br>
+ int x; // expected-warning {{non-static data member 'x' of 'J' shadows member inherited from type 'I'}}<br>
+};<br>
+<br>
+// non-access paths<br>
+class N : W {<br>
+};<br>
+<br>
+struct K {<br>
+ int y;<br>
+};<br>
+<br>
+struct L : private K {<br>
+};<br>
+<br>
+struct M : L {<br>
+ int y;<br>
+ int w;<br>
+};<br>
+<br>
+// Multiple ambiguous paths with different accesses<br>
+struct A1 {<br>
+ int x; // expected-note {{declared here}}<br>
+};<br>
+<br>
+class B1 : A1 {<br>
+};<br>
+<br>
+struct B2 : A1 {<br>
+};<br>
+<br>
+struct C1 : B1, B2 {<br>
+};<br>
+<br>
+class D1 : C1 {<br>
+};<br>
+<br>
+struct D2 : C1 {<br>
+};<br>
+<br>
+class D3 : C1 {<br>
+};<br>
+<br>
+struct E1 : D1, D2, D3{<br>
+ int x; // expected-warning {{non-static data member 'x' of 'E1' shadows member inherited from type 'A1'}}<br>
+};<br>
+<br>
+<br>
+<br>
<br>
Modified: cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p6.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp?rev=294401&r1=294400&r2=294401&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/CXX/<wbr>class.derived/class.member.<wbr>lookup/p6.cpp?rev=294401&r1=<wbr>294400&r2=294401&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p6.cpp (original)<br>
+++ cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p6.cpp Tue Feb 7 21:30:13 2017<br>
@@ -1,24 +1,24 @@<br>
-// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wshadow-field<br>
<br>
class V {<br>
public:<br>
int f();<br>
- int x;<br>
+ int x; // expected-note {{declared here}}<br>
};<br>
<br>
class W {<br>
public:<br>
int g(); // expected-note{{member found by ambiguous name lookup}}<br>
- int y; // expected-note{{member found by ambiguous name lookup}}<br>
+ int y; // expected-note{{member found by ambiguous name lookup}} expected-note {{declared here}}<br>
};<br>
<br>
class B : public virtual V, public W<br>
{<br>
public:<br>
int f();<br>
- int x;<br>
+ int x; // expected-warning {{non-static data member 'x' of 'B' shadows member inherited from type 'V'}}<br>
int g(); // expected-note{{member found by ambiguous name lookup}}<br>
- int y; // expected-note{{member found by ambiguous name lookup}}<br>
+ int y; // expected-note{{member found by ambiguous name lookup}} expected-warning {{non-static data member 'y' of 'B' shadows member inherited from type 'W'}}<br>
};<br>
<br>
class C : public virtual V, public W { };<br>
<br>
Modified: cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p7.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p7.cpp?rev=294401&r1=294400&r2=294401&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/CXX/<wbr>class.derived/class.member.<wbr>lookup/p7.cpp?rev=294401&r1=<wbr>294400&r2=294401&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p7.cpp (original)<br>
+++ cfe/trunk/test/CXX/class.<wbr>derived/class.member.lookup/<wbr>p7.cpp Tue Feb 7 21:30:13 2017<br>
@@ -1,11 +1,9 @@<br>
-// RUN: %clang_cc1 -verify %s<br>
+// RUN: %clang_cc1 -verify %s -Wshadow-field<br>
<br>
-// expected-no-diagnostics<br>
-<br>
-struct A { int n; };<br>
-struct B { float n; };<br>
+struct A { int n; }; // expected-note {{declared here}}<br>
+struct B { float n; }; // expected-note {{declared here}}<br>
struct C : A, B {};<br>
struct D : virtual C {};<br>
-struct E : virtual C { char n; };<br>
+struct E : virtual C { char n; }; // expected-warning {{non-static data member 'n' of 'E' shadows member inherited from type 'A'}} expected-warning {{non-static data member 'n' of 'E' shadows member inherited from type 'B'}}<br>
struct F : D, E {} f;<br>
char &k = f.n;<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>