From 2d8c760df75b6ae0a5efb41fda17af317a929602 Mon Sep 17 00:00:00 2001 From: John McCall Date: Sat, 20 Mar 2010 04:12:52 +0000 Subject: [PATCH] Implement -Wshadow for parameter declarations as well. llvm-svn: 99037 --- clang/lib/Sema/Sema.h | 2 +- clang/lib/Sema/SemaDecl.cpp | 54 ++++++++++++++++++++---------- clang/test/Sema/warn-shadow.c | 29 ++++++++++++++-- clang/test/SemaCXX/warn-shadow.cpp | 6 ++++ 4 files changed, 70 insertions(+), 21 deletions(-) diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h index aca4fdcb6183..be0fa3225783 100644 --- a/clang/lib/Sema/Sema.h +++ b/clang/lib/Sema/Sema.h @@ -784,7 +784,7 @@ public: const LookupResult &Previous, Scope *S); void DiagnoseFunctionSpecifiers(Declarator& D); - void DiagnoseShadow(NamedDecl* D, const LookupResult& R); + void DiagnoseShadow(Scope *S, Declarator &D, const LookupResult& R); NamedDecl* ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC, QualType R, TypeSourceInfo *TInfo, LookupResult &Previous, bool &Redeclaration); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 2e69108617e4..bab3d98e212e 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2404,7 +2404,8 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC, } // Diagnose shadowed variables before filtering for scope. - DiagnoseShadow(NewVD, Previous); + if (!D.getCXXScopeSpec().isSet()) + DiagnoseShadow(S, D, Previous); // Don't consider existing declarations that are in a different // scope and are out-of-semantic-context declarations (if the new @@ -2465,46 +2466,61 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC, /// For performance reasons, the lookup results are reused from the calling /// context. /// -/// \param D variable decl to diagnose. Must be a variable. -/// \param R cached previous lookup of \p D. +/// \param S the scope in which the shadowing name is being declared +/// \param R the lookup of the name /// -void Sema::DiagnoseShadow(NamedDecl* D, const LookupResult& R) { - assert(D->getKind() == Decl::Var && "Expecting variable."); - +void Sema::DiagnoseShadow(Scope *S, Declarator &D, + const LookupResult& R) { // Return if warning is ignored. if (Diags.getDiagnosticLevel(diag::warn_decl_shadow) == Diagnostic::Ignored) return; - // Return if not local decl. - if (!D->getDeclContext()->isFunctionOrMethod()) + // Don't diagnose declarations at file scope. The scope might not + // have a DeclContext if (e.g.) we're parsing a function prototype. + DeclContext *NewDC = static_cast(S->getEntity()); + if (NewDC && NewDC->isFileContext()) return; - - DeclarationName Name = D->getDeclName(); - - // Return if lookup has no result. + + // Only diagnose if we're shadowing an unambiguous field or variable. if (R.getResultKind() != LookupResult::Found) return; - // Return if not variable decl. NamedDecl* ShadowedDecl = R.getFoundDecl(); if (!isa(ShadowedDecl) && !isa(ShadowedDecl)) return; - // Determine kind of declaration. - DeclContext *DC = ShadowedDecl->getDeclContext(); + DeclContext *OldDC = ShadowedDecl->getDeclContext(); + + // Only warn about certain kinds of shadowing for class members. + if (NewDC && NewDC->isRecord()) { + // In particular, don't warn about shadowing non-class members. + if (!OldDC->isRecord()) + return; + + // TODO: should we warn about static data members shadowing + // static data members from base classes? + + // TODO: don't diagnose for inaccessible shadowed members. + // This is hard to do perfectly because we might friend the + // shadowing context, but that's just a false negative. + } + + // Determine what kind of declaration we're shadowing. unsigned Kind; - if (isa(DC)) { + if (isa(OldDC)) { if (isa(ShadowedDecl)) Kind = 3; // field else Kind = 2; // static data member - } else if (DC->isFileContext()) + } else if (OldDC->isFileContext()) Kind = 1; // global else Kind = 0; // local + DeclarationName Name = R.getLookupName(); + // Emit warning and note. - Diag(D->getLocation(), diag::warn_decl_shadow) << Name << Kind << DC; + Diag(R.getNameLoc(), diag::warn_decl_shadow) << Name << Kind << OldDC; Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration); } @@ -3968,6 +3984,8 @@ Sema::ActOnParamDeclarator(Scope *S, Declarator &D) { II = 0; D.SetIdentifier(0, D.getIdentifierLoc()); D.setInvalidType(true); + } else { + DiagnoseShadow(S, D, R); } } } diff --git a/clang/test/Sema/warn-shadow.c b/clang/test/Sema/warn-shadow.c index dacf66d5920b..c9a783b437a0 100644 --- a/clang/test/Sema/warn-shadow.c +++ b/clang/test/Sema/warn-shadow.c @@ -1,6 +1,6 @@ -// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow %s +// RUN: %clang_cc1 -verify -fsyntax-only -fblocks -Wshadow %s -int i; // expected-note {{previous declaration is here}} +int i; // expected-note 3 {{previous declaration is here}} void foo() { int pass1; @@ -18,3 +18,28 @@ void foo() { int sin; // okay; 'sin' has not been declared, even though it's a builtin. } + +// +void (^test1)(int) = ^(int i) { // expected-warning {{declaration shadows a variable in the global scope}} \ + // expected-note{{previous declaration is here}} + { + int i; // expected-warning {{declaration shadows a local variable}} \ + // expected-note{{previous declaration is here}} + + (^(int i) { return i; })(i); //expected-warning {{declaration shadows a local variable}} + } +}; + + +struct test2 { + int i; +}; + +void test3(void) { + struct test4 { + int i; + }; +} + +void test4(int i) { // expected-warning {{declaration shadows a variable in the global scope}} +} diff --git a/clang/test/SemaCXX/warn-shadow.cpp b/clang/test/SemaCXX/warn-shadow.cpp index c637f42ea986..509c34435560 100644 --- a/clang/test/SemaCXX/warn-shadow.cpp +++ b/clang/test/SemaCXX/warn-shadow.cpp @@ -36,3 +36,9 @@ class A { char *data; // expected-warning {{declaration shadows a static data member of 'A'}} } }; + +// TODO: this should warn, +class B : A { + int data; + static int field; +};