ahmedasadi updated this revision to Diff 93583. ahmedasadi marked an inline comment as done. ahmedasadi added a comment.
Re-ordered the checks in shouldWarnIfShadowedDecl as suggested by rnk. https://reviews.llvm.org/D31235 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaDecl.cpp test/SemaCXX/warn-shadow.cpp
Index: test/SemaCXX/warn-shadow.cpp =================================================================== --- test/SemaCXX/warn-shadow.cpp +++ test/SemaCXX/warn-shadow.cpp @@ -1,20 +1,27 @@ -// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow-all %s +// RUN: %clang_cc1 -verify -fsyntax-only -std=c++11 -Wshadow-all %s namespace { int i; // expected-note {{previous declaration is here}} } namespace one { namespace two { int j; // expected-note {{previous declaration is here}} + typedef int jj; // expected-note 2 {{previous declaration is here}} + using jjj=int; // expected-note 2 {{previous declaration is here}} } } namespace xx { int m; + typedef int mm; + using mmm=int; + } namespace yy { int m; + typedef char mm; + using mmm=char; } using namespace one::two; @@ -25,14 +32,19 @@ int i; // expected-warning {{declaration shadows a variable in namespace '(anonymous)'}} int j; // expected-warning {{declaration shadows a variable in namespace 'one::two'}} int m; + int mm; + int mmm; } class A { - static int data; // expected-note {{previous declaration}} - // expected-note@+1 {{previous declaration}} + static int data; // expected-note 1 {{previous declaration}} + // expected-note@+1 1 {{previous declaration}} int field; int f1, f2, f3, f4; // expected-note 8 {{previous declaration is here}} + typedef int a1; // expected-note 2 {{previous declaration}} + using a2=int; // expected-note 2 {{previous declaration}} + // The initialization is safe, but the modifications are not. A(int f1, int f2, int f3, int f4) // expected-note-re 4 {{variable 'f{{[0-4]}}' is declared here}} : f1(f1) { @@ -50,6 +62,28 @@ void test() { char *field; // expected-warning {{declaration shadows a field of 'A'}} char *data; // expected-warning {{declaration shadows a static data member of 'A'}} + char *a1; // no warning + char *a2; // no warning + char *jj; // no warning + char *jjj; // no warning + } + + void test2() { + typedef char field; // no warning + typedef char data; // no warning + typedef char a1; // expected-warning {{declaration shadows a typedef in 'A'}} + typedef char a2; // expected-warning {{declaration shadows a type alias in 'A'}} + typedef char jj; // expected-warning {{declaration shadows a typedef in namespace 'one::two'}} + typedef char jjj; // expected-warning {{declaration shadows a type alias in namespace 'one::two'}} + } + + void test3() { + using field=char; // no warning + using data=char; // no warning + using a1=char; // expected-warning {{declaration shadows a typedef in 'A'}} + using a2=char; // expected-warning {{declaration shadows a type alias in 'A'}} + using jj=char; // expected-warning {{declaration shadows a typedef in namespace 'one::two'}} + using jjj=char; // expected-warning {{declaration shadows a type alias in namespace 'one::two'}} } }; @@ -63,13 +97,23 @@ namespace rdar8900456 { struct Foo { static void Baz(); + static void Baz1(); + static void Baz2(); private: int Bar; }; void Foo::Baz() { double Bar = 12; // Don't warn. } + +void Foo::Baz1() { + typedef int Bar; // Don't warn. +} + +void Foo::Baz2() { + using Bar=int; // Don't warn. +} } // http://llvm.org/PR9160 @@ -87,23 +131,68 @@ }; } -extern int bob; // expected-note {{previous declaration is here}} +extern int bob; // expected-note 1 {{previous declaration is here}} +typedef int bob1; // expected-note 2 {{previous declaration is here}} +using bob2=int; // expected-note 2 {{previous declaration is here}} // rdar://8883302 void rdar8883302() { extern int bob; // don't warn for shadowing. } void test8() { int bob; // expected-warning {{declaration shadows a variable in the global namespace}} + int bob1; //no warning + int bob2; // no warning +} + +void test9() { + typedef int bob; // no warning + typedef int bob1; // expected-warning {{declaration shadows a typedef in the global namespace}} + typedef int bob2; // expected-warning {{declaration shadows a type alias in the global namespace}} +} + +void test10() { + using bob=int; // no warning + using bob1=int; // expected-warning {{declaration shadows a typedef in the global namespace}} + using bob2=int; // expected-warning {{declaration shadows a type alias 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'}} + + using c=char; // expected-note {{previous definition is here}} + using c=int; // expected-error {{type alias redefinition with different types ('int' vs 'char')}} + + typedef char d; // expected-note {{previous definition is here}} + typedef int d; // expected-error {{typedef redefinition with different types ('int' vs 'char')}} + + using e=char; // expected-note {{previous definition is here}} + typedef int e; // expected-error {{type alias redefinition with different types ('int' vs 'char')}} + + int f; // expected-note {{previous definition is here}} + using f=int; // expected-error {{redefinition of 'f'}} + + using g=int; // expected-note {{previous definition is here}} + int g; // expected-error {{redefinition of 'g'}} + + typedef int h; // expected-note {{previous definition is here}} + int h; // expected-error {{redefinition of 'h'}} + + int k; // expected-note {{previous definition is here}} + typedef int k; // expected-error {{redefinition of 'k'}} + + using l=char; // no warning or error. + using l=char; // no warning or error. + typedef char l; // no warning or error. + + typedef char n; // no warning or error. + typedef char n; // no warning or error. + using n=char; // no warning or error. } } Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -5530,6 +5530,10 @@ NamedDecl* Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD, LookupResult &Previous, bool &Redeclaration) { + + // Find the shadowed declaration before filtering for scope. + NamedDecl *ShadowedDecl = getShadowedDeclaration(NewTD, Previous); + // Merge the decl with the existing one if appropriate. If the decl is // in an outer scope, it isn't the same thing. FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage*/false, @@ -5540,6 +5544,9 @@ MergeTypedefNameDecl(S, NewTD, Previous); } + if (ShadowedDecl && !Redeclaration) + CheckShadow(NewTD, ShadowedDecl, Previous); + // If this is the C FILE type, notify the AST context. if (IdentifierInfo *II = NewTD->getIdentifier()) if (!NewTD->isInvalidDecl() && @@ -6671,13 +6678,18 @@ } /// Enum describing the %select options in diag::warn_decl_shadow. -enum ShadowedDeclKind { SDK_Local, SDK_Global, SDK_StaticMember, SDK_Field }; +enum ShadowedDeclKind { SDK_Local, SDK_Global, SDK_StaticMember, SDK_Field, SDK_Typedef, SDK_Using }; /// Determine what kind of declaration we're shadowing. static ShadowedDeclKind computeShadowedDeclKind(const NamedDecl *ShadowedDecl, const DeclContext *OldDC) { - if (isa<RecordDecl>(OldDC)) + if (isa<TypeAliasDecl>(ShadowedDecl)) + return SDK_Using; + else if (isa<TypedefDecl>(ShadowedDecl)) + return SDK_Typedef; + else if (isa<RecordDecl>(OldDC)) return isa<FieldDecl>(ShadowedDecl) ? SDK_Field : SDK_StaticMember; + return OldDC->isFileContext() ? SDK_Global : SDK_Local; } @@ -6692,28 +6704,44 @@ return SourceLocation(); } +static bool shouldWarnIfShadowedDecl(const DiagnosticsEngine &Diags, + const LookupResult &R) { + // Only diagnose if we're shadowing an unambiguous field or variable. + if (R.getResultKind() != LookupResult::Found) + return false; + + // Return false if warning is ignored. + return !Diags.isIgnored(diag::warn_decl_shadow, R.getNameLoc()); +} + /// \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())) + if (!shouldWarnIfShadowedDecl(Diags, R)) return nullptr; // Don't diagnose declarations at file scope. if (D->hasGlobalStorage()) return nullptr; - // Only diagnose if we're shadowing an unambiguous field or variable. - if (R.getResultKind() != LookupResult::Found) - return nullptr; - NamedDecl *ShadowedDecl = R.getFoundDecl(); return isa<VarDecl>(ShadowedDecl) || isa<FieldDecl>(ShadowedDecl) ? ShadowedDecl : nullptr; } +/// \brief Return the declaration shadowed by the given typedef \p D, or null +/// if it doesn't shadow any declaration or shadowing warnings are disabled. +NamedDecl *Sema::getShadowedDeclaration(const TypedefNameDecl *D, + const LookupResult &R) { + if (!shouldWarnIfShadowedDecl(Diags, R)) + return nullptr; + + NamedDecl *ShadowedDecl = R.getFoundDecl(); + return isa<TypedefNameDecl>(ShadowedDecl) ? ShadowedDecl : nullptr; +} + /// \brief Diagnose variable or built-in function shadowing. Implements /// -Wshadow. /// @@ -6723,7 +6751,7 @@ /// \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, +void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl, const LookupResult &R) { DeclContext *NewDC = D->getDeclContext(); @@ -6735,13 +6763,13 @@ // Fields shadowed by constructor parameters are a special case. Usually // the constructor initializes the field with the parameter. - if (isa<CXXConstructorDecl>(NewDC) && isa<ParmVarDecl>(D)) { - // Remember that this was shadowed so we can either warn about its - // modification or its existence depending on warning settings. - D = D->getCanonicalDecl(); - ShadowingDecls.insert({D, FD}); - return; - } + if (isa<CXXConstructorDecl>(NewDC)) + if (const auto PVD = dyn_cast<ParmVarDecl>(D)) { + // Remember that this was shadowed so we can either warn about its + // modification or its existence depending on warning settings. + ShadowingDecls.insert({PVD->getCanonicalDecl(), FD}); + return; + } } if (VarDecl *shadowedVar = dyn_cast<VarDecl>(ShadowedDecl)) @@ -6759,7 +6787,8 @@ unsigned WarningDiag = diag::warn_decl_shadow; SourceLocation CaptureLoc; - if (isa<VarDecl>(ShadowedDecl) && NewDC && isa<CXXMethodDecl>(NewDC)) { + if (isa<VarDecl>(D) && isa<VarDecl>(ShadowedDecl) && 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) { @@ -6773,7 +6802,8 @@ // 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, cast<VarDecl>(ShadowedDecl)}); + ->ShadowingDecls.push_back({cast<VarDecl>(D), + cast<VarDecl>(ShadowedDecl)}); return; } } Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -1735,8 +1735,9 @@ static bool adjustContextForLocalExternDecl(DeclContext *&DC); void DiagnoseFunctionSpecifiers(const DeclSpec &DS); + NamedDecl *getShadowedDeclaration(const TypedefNameDecl *D, const LookupResult &R); NamedDecl *getShadowedDeclaration(const VarDecl *D, const LookupResult &R); - void CheckShadow(VarDecl *D, NamedDecl *ShadowedDecl, const LookupResult &R); + void CheckShadow(NamedDecl *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 Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -369,7 +369,9 @@ "local variable|" "variable in %2|" "static data member of %2|" - "field of %2}1">, + "field of %2|" + "typedef in %2|" + "type alias in %2}1">, InGroup<Shadow>, DefaultIgnore; def warn_decl_shadow_uncaptured_local : Warning<warn_decl_shadow.Text>,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits