diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h
index dd4443e5dd1f3cba775dd1987787195292f96b1f..085faeae4834c66f6b4e6498b30eef7a1b56b0d3 100644
--- a/include/clang/Basic/Diagnostic.h
+++ b/include/clang/Basic/Diagnostic.h
@@ -1072,10 +1072,10 @@ inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
// 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);
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 8b5d8df6fc4f29daa033b2e4e4e005e74549ec76..7b4740fe22a2fb323de92c3c59a92dafa354be8d 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -337,7 +337,16 @@ def SelfMove : DiagGroup<"self-move">;
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">;
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index b04abc55afbb91e3fcd4cfbac189061c1a3615df..cd7a7557d073bae61b4f6ca02548591f8fdd5bbf 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -352,6 +352,14 @@ def warn_decl_shadow :
"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<
@@ -1667,7 +1675,7 @@ def warn_maybe_uninit_var : Warning<
"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<
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index f53bc05cc1e7afddcdaf16b102abeed4c9c7d758..14f9d2c86fc9baaf5d0ab4f29fb1cfc43cf88e13 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1653,6 +1653,17 @@ public:
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,
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index fcb12f6cebf986e0380d271b9509614ac102a796..3f2c41b67bcc0ce0d48519d3084137e6962b2a2e 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -889,7 +889,7 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
// 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;
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 03f235d4399c20a047f6ac5a88236b7ba66d1873..0c8781c023383f28ce81d2180284ee55da68dc48 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1609,9 +1609,19 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
// 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);
+ }
}
}
@@ -6382,6 +6392,17 @@ Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
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.
///
@@ -6410,12 +6431,23 @@ void Sema::CheckShadow(Scope *S, VarDecl *D, const LookupResult& R) {
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
@@ -6443,27 +6475,13 @@ void Sema::CheckShadow(Scope *S, VarDecl *D, const LookupResult& R) {
// 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);
}
@@ -6479,6 +6497,30 @@ void Sema::CheckShadow(Scope *S, VarDecl *D) {
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>
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 065c236d6cc5058fc70d703e84e4f4bbdad90c63..6b043368db1b2f483123dcf2e205be043002cf62 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -9733,6 +9733,9 @@ static void DiagnoseConstAssignment(Sema &S, const Expr *E,
/// 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);
diff --git a/test/SemaCXX/warn-shadow.cpp b/test/SemaCXX/warn-shadow.cpp
index 5ad2233d234a0328b3e86a7c8dfe98d6004f3d3f..9d68fe7c47a666f93fd277dd541dba6edfe953ab 100644
--- a/test/SemaCXX/warn-shadow.cpp
+++ b/test/SemaCXX/warn-shadow.cpp
@@ -1,4 +1,4 @@
-// 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}}
@@ -29,7 +29,23 @@ void foo() {
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'}}