Warn on self-initializations

[rtrieu]

Summary

Improve some diagnostics around self-initialization. The uninitialized checker does not warn on self-initialization, but does warn later if the variable is used. GCC also has a -Winit-self which will enable a -Wuninitialized warning for self-initialized. -Winit-self is active in -Wall

There's also the matter of the variable being const. GCC -Wuninitialized will warn on a const self-initialized with or without -Winit-self.

This patch will make it so that const self-initialization is always warned on under -Wuninitialized. Since Clang does not use warning flags to control other warnings, -Winit-self will be a new warning group for non-const self-initialized. -Winit-self will also be in -Wall via -Wmost.

// GCC - warn with -Wuninitialized and -Winit-self
// Clang - no warning
// Clang with patch - warn with -Winit-self
void f1() {
  int x = x;
}

// GCC - warn with -Wuninitialized
// Clang - no warning
// Clang with patch - warn with -Wuninitialized
void f2() {
  const int x = x;
}

include/clang/Basic/DiagnosticGroups.td

 def IncompleteModule : DiagGroup<"incomplete-module",
     [IncompleteUmbrella, NonModularIncludeInModule]>;
 def PrivateModule : DiagGroup<"private-module">;
 
 def CXX11InlineNamespace : DiagGroup<"c++11-inline-namespace">;
 def InvalidNoreturn : DiagGroup<"invalid-noreturn">;
 def InvalidSourceEncoding : DiagGroup<"invalid-source-encoding">;
 def KNRPromotedParameter : DiagGroup<"knr-promoted-parameter">;
-def : DiagGroup<"init-self">;
+def InitSelf : DiagGroup<"init-self">;
 def : DiagGroup<"inline">;
 def : DiagGroup<"invalid-pch">;
 def GNULabelsAsValue : DiagGroup<"gnu-label-as-value">;
 def LiteralRange : DiagGroup<"literal-range">;
 def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
                                       [CXX98CompatLocalTypeTemplateArgs]>;
 def RangeLoopAnalysis : DiagGroup<"range-loop-analysis">;
 def ForLoopAnalysis : DiagGroup<"for-loop-analysis">;

 def Most : DiagGroup<"most", [
     CharSubscript,
     Comment,
     DeleteNonVirtualDtor,
     ForLoopAnalysis,
     Format,
     Implicit,
     InfiniteRecursion,
+    InitSelf,
     MismatchedTags,
     MissingBraces,
     Move,
     MultiChar,
     Reorder,
     ReturnType,
     SelfAssignment,
     SelfMove,

include/clang/Basic/DiagnosticSemaKinds.td

def note_uninit_in_this_constructor : Note<
   "during field initialization in %select{this|the implicit default}0 "
   "constructor">;
 def warn_static_self_reference_in_init : Warning<
   "static variable %0 is suspiciously used within its own initialization">,
   InGroup<UninitializedStaticSelfInit>;
 def warn_uninit_self_reference_in_init : Warning<
   "variable %0 is uninitialized when used within its own initialization">,
   InGroup<Uninitialized>;
+def warn_uninit_self_init : Warning<"variable %0 is initialized with itself">,
+  InGroup<InitSelf>, DefaultIgnore;
 def warn_uninit_self_reference_in_reference_init : Warning<
   "reference %0 is not yet bound to a value when used within its own"
   " initialization">,
   InGroup<Uninitialized>;
 def warn_uninit_var : Warning<
   "variable %0 is uninitialized when %select{used here|captured by block}1">,
   InGroup<Uninitialized>, DefaultIgnore;
 def warn_sometimes_uninit_var : Warning<

lib/Sema/AnalysisBasedWarnings.cpp

if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Use.getUser())) {
     // form of:
     //
     //   int x = x;
     //
     // This is used to indicate to GCC that 'x' is intentionally left
     // uninitialized. Proven code paths which access 'x' in
     // an uninitialized state after this will still warn.
     if (const Expr *Initializer = VD->getInit()) {
-      if (!alwaysReportSelfInit && DRE == Initializer->IgnoreParenImpCasts())
+      if (!VD->getType().isConstQualified() && !alwaysReportSelfInit &&
+          DRE == Initializer->IgnoreParenImpCasts())
         return false;
 
       ContainsReference CR(S.Context, DRE);
       CR.Visit(Initializer);
       if (CR.doesContainReference()) {
         S.Diag(DRE->getBeginLoc(), diag::warn_uninit_self_reference_in_init)
             << VD->getDeclName() << VD->getLocation() << DRE->getSourceRange();
         return true;
public:
   void flushDiagnostics() {
     for (const auto &P : uses) {
       const VarDecl *vd = P.first;
       const MappedType &V = P.second;
 
       UsesVec *vec = V.getPointer();
       bool hasSelfInit = V.getInt();
 
+      if (hasSelfInit && vd->getType().isConstQualified()) {
+        // Handle "const int a = a;"
+        DiagnoseUninitializedUse(S, vd,
+                                 UninitUse(vd->getInit()->IgnoreParenCasts(),
+                                           /* isAlwaysUninit */ true),
+                                 /* alwaysReportSelfInit */ true);
+      } else if (!vec->empty() && hasSelfInit && hasAlwaysUninitializedUse(vec))
         // Specially handle the case where we have uses of an uninitialized
         // variable, but the root cause is an idiomatic self-init.  We want
-      // to report the diagnostic at the self-init since that is the root cause.
-      if (!vec->empty() && hasSelfInit && hasAlwaysUninitializedUse(vec))
+        // to report the diagnostic at the self-init since that is the root
+        // cause.
         DiagnoseUninitializedUse(S, vd,
                                  UninitUse(vd->getInit()->IgnoreParenCasts(),
                                            /* isAlwaysUninit */ true),
                                  /* alwaysReportSelfInit */ true);
       else {
         // Sort the uses by their SourceLocations.  While not strictly
         // guaranteed to produce them in line/column order, this will provide
         // a stable ordering.

lib/Sema/SemaDecl.cpp

if (!VDecl->isInvalidDecl()) {
     if (Result.isInvalid()) {
       VDecl->setInvalidDecl();
       return;
     }
 
     Init = Result.getAs<Expr>();
   }
 
+  if (!VDecl->getType().isConstQualified()) {
+    if (auto *SubExpr = dyn_cast<ImplicitCastExpr>(Init->IgnoreParens())) {
+      if (SubExpr->getCastKind() == CK_LValueToRValue) {
+        if (auto *DRE = dyn_cast<DeclRefExpr>(SubExpr->getSubExpr())) {
+          if (DRE->getDecl() == VDecl) {
+            Diag(VDecl->getLocation(), diag::warn_uninit_self_init)
+                << VDecl << VDecl->getSourceRange() << DRE->getSourceRange();
+          }
+        }
+      }
+    }
+  }
+
   // Check for self-references within variable initializers.
   // Variables declared within a function/method body (except for references)
   // are handled by a dataflow analysis.
   if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() ||
       VDecl->getType()->isReferenceType()) {
     CheckSelfReference(*this, RealDecl, Init, DirectInit);
   }
 

test/SemaCXX/uninitialized.cpp

-// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value -Wno-unused-lambda-capture -std=c++1z -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wno-unused -std=c++1z -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wno-unused -Wno-init-self -std=c++1z -verify %s
 
 // definitions for std::move
 namespace std {
 inline namespace foo {
 template <class T> struct remove_reference { typedef T type; };
 template <class T> struct remove_reference<T&> { typedef T type; };
 template <class T> struct remove_reference<T&&> { typedef T type; };
 
class A {
   A(int (*) [6]) : a(rvalueref::move(a)) {}
   A(int (*) [7]) : a(rvalueref::notmove(a)) {}
 };
 }
 
 void array_capture(bool b) {
   const char fname[] = "array_capture";
   if (b) {
-    int unused; // expected-warning {{unused variable}}
+    int unused;
   } else {
     [fname]{};
   }
 }
 
 void if_switch_init_stmt(int k) {
   if (int n = 0; (n == k || k > 5)) {}
 
   if (int n; (n == k || k > 5)) {} // expected-warning {{uninitialized}} expected-note {{initialize}}
 
   switch (int n = 0; (n == k || k > 5)) {} // expected-warning {{boolean}}
 
   switch (int n; (n == k || k > 5)) {} // expected-warning {{uninitialized}} expected-note {{initialize}} expected-warning {{boolean}}
 }
+
+namespace const_self_init {
+const int a = a;  // no-warning for global
+
+void const_init() {
+  const int a = a; // expected-warning {{variable 'a' is uninitialized when used within its own initialization}}
+
+  const int b = b; // expected-warning {{variable 'b' is uninitialized when used within its own initialization}}
+  int c = b;
+}
+
+}