Author: aaronballman Date: Fri Nov 2 14:04:44 2018 New Revision: 346041 URL: http://llvm.org/viewvc/llvm-project?rev=346041&view=rev Log: Diagnose parameter names that shadow the names of inherited fields under -Wshadow-field.
This addresses PR34120. Note, unlike GCC, we take into account the accessibility of the field when deciding whether to warn or not. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/test/SemaCXX/warn-shadow.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=346041&r1=346040&r2=346041&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov 2 14:04:44 2018 @@ -9467,16 +9467,15 @@ def warn_block_literal_qualifiers_on_omi InGroup<IgnoredQualifiers>; def ext_warn_gnu_final : ExtWarn< - "__final is a GNU extension, consider using C++11 final">, - InGroup<GccCompat>; - -def warn_shadow_field : - Warning<"non-static data member %0 of %1 shadows member inherited from " - "type %2">, - InGroup<ShadowField>, DefaultIgnore; -def note_shadow_field : Note<"declared here">; - -def err_multiversion_required_in_redecl : Error< + "__final is a GNU extension, consider using C++11 final">, + InGroup<GccCompat>; + +def warn_shadow_field : Warning< + "%select{parameter|non-static data member}3 %0 %select{|of %1 }3shadows " + "member inherited from type %2">, InGroup<ShadowField>, DefaultIgnore; +def note_shadow_field : Note<"declared here">; + +def err_multiversion_required_in_redecl : Error< "function declaration is missing %select{'target'|'cpu_specific' or " "'cpu_dispatch'}0 attribute in a multiversioned function">; def note_multiversioning_caused_here : Note< Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=346041&r1=346040&r2=346041&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Fri Nov 2 14:04:44 2018 @@ -10588,7 +10588,8 @@ private: /// Check if there is a field shadowing. void CheckShadowInheritedFields(const SourceLocation &Loc, DeclarationName FieldName, - const CXXRecordDecl *RD); + const CXXRecordDecl *RD, + bool DeclIsField = true); /// Check if the given expression contains 'break' or 'continue' /// statement that produces control flow different from GCC. Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=346041&r1=346040&r2=346041&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Nov 2 14:04:44 2018 @@ -12366,6 +12366,13 @@ Decl *Sema::ActOnParamDeclarator(Scope * D.setInvalidType(true); } } + + if (LangOpts.CPlusPlus) { + DeclarationNameInfo DNI = GetNameForDeclarator(D); + if (auto *RD = dyn_cast<CXXRecordDecl>(CurContext)) + CheckShadowInheritedFields(DNI.getLoc(), DNI.getName(), RD, + /*DeclIsField*/ false); + } } // Temporarily put parameter variables in the translation unit, not Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=346041&r1=346040&r2=346041&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Nov 2 14:04:44 2018 @@ -2832,13 +2832,14 @@ static const ParsedAttr *getMSPropertyAt return nullptr; } -// Check if there is a field shadowing. -void Sema::CheckShadowInheritedFields(const SourceLocation &Loc, - DeclarationName FieldName, - const CXXRecordDecl *RD) { - if (Diags.isIgnored(diag::warn_shadow_field, Loc)) - return; - +// Check if there is a field shadowing. +void Sema::CheckShadowInheritedFields(const SourceLocation &Loc, + DeclarationName FieldName, + const CXXRecordDecl *RD, + bool DeclIsField) { + if (Diags.isIgnored(diag::warn_shadow_field, Loc)) + return; + // To record a shadowed field in a base std::map<CXXRecordDecl*, NamedDecl*> Bases; auto FieldShadowed = [&](const CXXBaseSpecifier *Specifier, @@ -2872,13 +2873,13 @@ void Sema::CheckShadowInheritedFields(co continue; auto BaseField = It->second; assert(BaseField->getAccess() != AS_private); - if (AS_none != - CXXRecordDecl::MergeAccess(P.Access, BaseField->getAccess())) { - Diag(Loc, diag::warn_shadow_field) - << FieldName << RD << Base; - Diag(BaseField->getLocation(), diag::note_shadow_field); - Bases.erase(It); - } + if (AS_none != + CXXRecordDecl::MergeAccess(P.Access, BaseField->getAccess())) { + Diag(Loc, diag::warn_shadow_field) + << FieldName << RD << Base << DeclIsField; + Diag(BaseField->getLocation(), diag::note_shadow_field); + Bases.erase(It); + } } } Modified: cfe/trunk/test/SemaCXX/warn-shadow.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-shadow.cpp?rev=346041&r1=346040&r2=346041&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/warn-shadow.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-shadow.cpp Fri Nov 2 14:04:44 2018 @@ -59,13 +59,13 @@ class A { // expected-warning-re@+1 4 {{constructor parameter 'f{{[0-4]}}' shadows the field 'f{{[0-9]}}' of 'A'}} A(int f1, int f2, int f3, int f4, double overload_dummy) {} - void test() { - char *field; // expected-warning {{declaration shadows a field of 'A'}} - char *data; // expected-warning {{declaration shadows a static data member of 'A'}} + 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 + char *a2; // no warning + char *jj; // no warning + char *jjj; // no warning } void test2() { @@ -196,14 +196,14 @@ void avoidWarningWhenRedefining(int b) { 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. + 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. -} + typedef char n; // no warning or error. + using n=char; // no warning or error. +} } @@ -219,9 +219,37 @@ void f(int a) { struct A { void g(int a) {} A() { int a; } - }; -} -} + }; +} +} + +namespace PR34120 { +struct A { + int B; // expected-note {{declared here}} +}; + +class C : public A { + void D(int B) {} // expected-warning {{parameter 'B' shadows member inherited from type 'A'}} + void E() { + extern void f(int B); // Ok + } +}; + +class Private { + int B; +}; +class Derived : Private { + void D(int B) {} // Ok +}; + +struct Static { + static int B; +}; + +struct Derived2 : Static { + void D(int B) {} +}; +} int PR24718; enum class X { PR24718 }; // Ok, not shadowing _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits