// so that we only match those arguments that are (statically) DeclContexts;
// other arguments that derive from DeclContext (e.g., RecordDecls) will not
// match.
-template<typename T>
-inline
-typename std::enable_if<std::is_same<T, DeclContext>::value,
- const DiagnosticBuilder &>::type
+template <typename T>
+inline typename std::enable_if<
+ std::is_same<typename std::remove_const<T>::type, DeclContext>::value,
+ const DiagnosticBuilder &>::type
operator<<(const DiagnosticBuilder &DB, T *DC) {
DB.AddTaggedVal(reinterpret_cast<intptr_t>(DC),
DiagnosticsEngine::ak_declcontext);
def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;
def Sentinel : DiagGroup<"sentinel">;
def MissingMethodReturnType : DiagGroup<"missing-method-return-type">;
-def Shadow : DiagGroup<"shadow">;
+
+def ShadowFieldInConstructorModified : DiagGroup<"shadow-field-in-constructor-modified">;
+def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-constructor",
+ [ShadowFieldInConstructorModified]>;
+
+// -Wshadow-all is a catch-all for all shadowing. -Wshadow is just the
+// shadowing that we think is unsafe.
+def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified]>;
+def ShadowAll : DiagGroup<"shadow-all", [Shadow, ShadowFieldInConstructor]>;
+
def Shorten64To32 : DiagGroup<"shorten-64-to-32">;
def : DiagGroup<"sign-promo">;
def SignCompare : DiagGroup<"sign-compare">;
"static data member of %2|"
"field of %2}1">,
InGroup<Shadow>, DefaultIgnore;
+def warn_ctor_parm_shadows_field:
+ Warning<"constructor parameter %0 shadows the field %1 of %2">,
+ InGroup<ShadowFieldInConstructor>, DefaultIgnore;
+def warn_modifying_shadowing_decl :
+ Warning<"modifying constructor parameter %0 that shadows a "
+ "field of %1">,
+ InGroup<ShadowFieldInConstructorModified>, DefaultIgnore;
+
// C++ using declarations
def err_using_requires_qualname : Error<
"variable %0 may be uninitialized when "
"%select{used here|captured by block}1">,
InGroup<UninitializedMaybe>, DefaultIgnore;
-def note_uninit_var_def : Note<"variable %0 is declared here">;
+def note_var_declared_here : Note<"variable %0 is declared here">;
def note_uninit_var_use : Note<
"%select{uninitialized use occurs|variable is captured by block}0 here">;
def warn_uninit_byref_blockvar_captured_by_block : Warning<
void DiagnoseFunctionSpecifiers(const DeclSpec &DS);
void CheckShadow(Scope *S, VarDecl *D, const LookupResult& R);
void CheckShadow(Scope *S, VarDecl *D);
+
+ /// Warn if 'E', which is an expression that is about to be modified, refers
+ /// to a shadowing declaration.
+ void CheckShadowingDeclModification(Expr *E, SourceLocation Loc);
+
+private:
+ /// Map of current shadowing declarations to shadowed declarations. Warn if
+ /// it looks like the user is trying to modify the shadowing declaration.
+ llvm::DenseMap<const NamedDecl *, const NamedDecl *> ShadowingDecls;
+
+public:
void CheckCastAlign(Expr *Op, QualType T, SourceRange TRange);
void handleTagNumbering(const TagDecl *Tag, Scope *TagScope);
void setTagNameForLinkagePurposes(TagDecl *TagFromDeclSpec,
// the initializer of that declaration & we didn't already suggest
// an initialization fixit.
if (!SuggestInitializationFixit(S, VD))
- S.Diag(VD->getLocStart(), diag::note_uninit_var_def)
+ S.Diag(VD->getLocStart(), diag::note_var_declared_here)
<< VD->getDeclName();
return true;
// If this was a forward reference to a label, verify it was defined.
if (LabelDecl *LD = dyn_cast<LabelDecl>(D))
CheckPoppedLabel(LD, *this);
-
- // Remove this name from our lexical scope.
+
+ // Remove this name from our lexical scope, and warn on it if we haven't
+ // already.
IdResolver.RemoveDecl(D);
+ auto ShadowI = ShadowingDecls.find(D);
+ if (ShadowI != ShadowingDecls.end()) {
+ if (const auto *FD = dyn_cast<FieldDecl>(ShadowI->second)) {
+ Diag(D->getLocation(), diag::warn_ctor_parm_shadows_field)
+ << D << FD << FD->getParent();
+ Diag(FD->getLocation(), diag::note_previous_declaration);
+ }
+ ShadowingDecls.erase(ShadowI);
+ }
}
}
return NewVD;
}
+/// Enum describing the %select options in diag::warn_decl_shadow.
+enum ShadowedDeclKind { SDK_Local, SDK_Global, SDK_StaticMember, SDK_Field };
+
+/// Determine what kind of declaration we're shadowing.
+static ShadowedDeclKind computeShadowedDeclKind(const NamedDecl *ShadowedDecl,
+ const DeclContext *OldDC) {
+ if (isa<RecordDecl>(OldDC))
+ return isa<FieldDecl>(ShadowedDecl) ? SDK_Field : SDK_StaticMember;
+ return OldDC->isFileContext() ? SDK_Global : SDK_Local;
+}
+
/// \brief Diagnose variable or built-in function shadowing. Implements
/// -Wshadow.
///
if (!isa<VarDecl>(ShadowedDecl) && !isa<FieldDecl>(ShadowedDecl))
return;
- // Fields are not shadowed by variables in C++ static methods.
- if (isa<FieldDecl>(ShadowedDecl))
+ if (FieldDecl *FD = dyn_cast<FieldDecl>(ShadowedDecl)) {
+ // Fields are not shadowed by variables in C++ static methods.
if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewDC))
if (MD->isStatic())
return;
+ // 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 (VarDecl *shadowedVar = dyn_cast<VarDecl>(ShadowedDecl))
if (shadowedVar->isExternC()) {
// For shadowing external vars, make sure that we point to the global
// shadowing context, but that's just a false negative.
}
- // Determine what kind of declaration we're shadowing.
-
- // The order must be consistent with the %select in the warning message.
- enum ShadowedDeclKind { Local, Global, StaticMember, Field };
- ShadowedDeclKind Kind;
- if (isa<RecordDecl>(OldDC)) {
- if (isa<FieldDecl>(ShadowedDecl))
- Kind = Field;
- else
- Kind = StaticMember;
- } else if (OldDC->isFileContext()) {
- Kind = Global;
- } else {
- Kind = Local;
- }
DeclarationName Name = R.getLookupName();
// Emit warning and note.
if (getSourceManager().isInSystemMacro(R.getNameLoc()))
return;
+ ShadowedDeclKind Kind = computeShadowedDeclKind(ShadowedDecl, OldDC);
Diag(R.getNameLoc(), diag::warn_decl_shadow) << Name << Kind << OldDC;
Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
}
CheckShadow(S, D, R);
}
+/// Check if 'E', which is an expression that is about to be modified, refers
+/// to a constructor parameter that shadows a field.
+void Sema::CheckShadowingDeclModification(Expr *E, SourceLocation Loc) {
+ // Quickly ignore expressions that can't be shadowing ctor parameters.
+ if (!getLangOpts().CPlusPlus || ShadowingDecls.empty())
+ return;
+ E = E->IgnoreParenImpCasts();
+ auto *DRE = dyn_cast<DeclRefExpr>(E);
+ if (!DRE)
+ return;
+ const NamedDecl *D = cast<NamedDecl>(DRE->getDecl()->getCanonicalDecl());
+ auto I = ShadowingDecls.find(D);
+ if (I == ShadowingDecls.end())
+ return;
+ const NamedDecl *ShadowedDecl = I->second;
+ const DeclContext *OldDC = ShadowedDecl->getDeclContext();
+ Diag(Loc, diag::warn_modifying_shadowing_decl) << D << OldDC;
+ Diag(D->getLocation(), diag::note_var_declared_here) << D;
+ Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
+
+ // Avoid issuing multiple warnings about the same decl.
+ ShadowingDecls.erase(I);
+}
+
/// Check for conflict between this global or extern "C" declaration and
/// previous global or extern "C" declarations. This is only used in C++.
template<typename T>
/// emit an error and return true. If so, return false.
static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
assert(!E->hasPlaceholderType(BuiltinType::PseudoObject));
+
+ S.CheckShadowingDeclModification(E, Loc);
+
SourceLocation OrigLoc = Loc;
Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context,
&Loc);
-// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow %s
+// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow-all %s
namespace {
int i; // expected-note {{previous declaration is here}}
class A {
static int data; // expected-note {{previous declaration}}
- int field; // expected-note {{previous declaration}}
+ // expected-note@+1 {{previous declaration}}
+ int field;
+ int f1, f2, f3, f4; // expected-note 8 {{previous declaration is here}}
+
+ // 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) {
+ f1 = 3; // expected-warning {{modifying constructor parameter 'f1' that shadows a field of 'A'}}
+ f1 = 4; // one warning per shadow
+ f2++; // expected-warning {{modifying constructor parameter 'f2' that shadows a field of 'A'}}
+ --f3; // expected-warning {{modifying constructor parameter 'f3' that shadows a field of 'A'}}
+ f4 += 2; // expected-warning {{modifying constructor parameter 'f4' that shadows a field of 'A'}}
+ }
+
+ // The initialization is safe, but the modifications are not.
+ // 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'}}
projects / clang / commitdiff