Tweak -Wuninitialized's handling of 'int x = x' to report that as the root cause of an uninitialized variable IFF there are other uses of that uninitialized variable. Fixes <rdar://problem/9259237>.

llvm-svn: 141881

clang/include/clang/Analysis/Analyses/UninitializedValues.h

@@ -27,10 +27,16 @@ class UninitVariablesHandler {
 public:
   UninitVariablesHandler() {}
   virtual ~UninitVariablesHandler();
-  
+
+  /// Called when the uninitialized variable is used at the given expression.
   virtual void handleUseOfUninitVariable(const Expr *ex,
                                          const VarDecl *vd,
                                          bool isAlwaysUninit) {}
+  
+  /// Called when the uninitialized variable analysis detects the
+  /// idiom 'int x = x'.  All other uses of 'x' within the initializer
+  /// are handled by handleUseOfUninitVariable.
+  virtual void handleSelfInit(const VarDecl *vd) {}
 };
 
 struct UninitVariablesAnalysisStats {

clang/lib/Analysis/UninitializedValues.cpp

@@ -484,11 +484,17 @@ void TransferFunctions::VisitDeclStmt(DeclStmt *ds) {
               vals[vd] = Uninitialized;
               lastLoad = 0;
               lastDR = 0;
+              if (handler)
+                handler->handleSelfInit(vd);
               return;
             }
           }
 
           // All other cases: treat the new variable as initialized.
+          // This is a minor optimization to reduce the propagation
+          // of the analysis, since we will have already reported
+          // the use of the uninitialized value (which visiting the
+          // initializer).
           vals[vd] = Initialized;
         }
       }

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -470,7 +470,8 @@ static bool SuggestInitializationFixit(Sema &S, const VarDecl *VD) {
 /// as a warning. If a pariticular use is one we omit warnings for, returns
 /// false.
 static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
-                                     const Expr *E, bool isAlwaysUninit) {
+                                     const Expr *E, bool isAlwaysUninit,
+                                     bool alwaysReportSelfInit = false) {
   bool isSelfInit = false;
 
   if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
@@ -490,7 +491,7 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
       // TODO: Should we suppress maybe-uninitialized warnings for
       // variables initialized in this way?
       if (const Expr *Initializer = VD->getInit()) {
-        if (DRE == Initializer->IgnoreParenImpCasts())
+        if (!alwaysReportSelfInit && DRE == Initializer->IgnoreParenImpCasts())
           return false;
 
         ContainsReference CR(S.Context, DRE);
@@ -541,7 +542,7 @@ struct SLocSort {
 class UninitValsDiagReporter : public UninitVariablesHandler {
   Sema &S;
   typedef SmallVector<UninitUse, 2> UsesVec;
-  typedef llvm::DenseMap<const VarDecl *, UsesVec*> UsesMap;
+  typedef llvm::DenseMap<const VarDecl *, std::pair<UsesVec*, bool> > UsesMap;
   UsesMap *uses;
   
 public:
@@ -549,17 +550,26 @@ public:
   ~UninitValsDiagReporter() { 
     flushDiagnostics();
   }
-  
+
-  void handleUseOfUninitVariable(const Expr *ex, const VarDecl *vd,
+  std::pair<UsesVec*, bool> &getUses(const VarDecl *vd) {
-                                 bool isAlwaysUninit) {
     if (!uses)
       uses = new UsesMap();
-    
+
-    UsesVec *&vec = (*uses)[vd];
+    UsesMap::mapped_type &V = (*uses)[vd];
+    UsesVec *&vec = V.first;
     if (!vec)
       vec = new UsesVec();
     
-    vec->push_back(std::make_pair(ex, isAlwaysUninit));
+    return V;
+  }
+  
+  void handleUseOfUninitVariable(const Expr *ex, const VarDecl *vd,
+                                 bool isAlwaysUninit) {
+    getUses(vd).first->push_back(std::make_pair(ex, isAlwaysUninit));
+  }
+  
+  void handleSelfInit(const VarDecl *vd) {
+    getUses(vd).second = true;    
   }
   
   void flushDiagnostics() {
@@ -568,22 +578,34 @@ public:
     
     for (UsesMap::iterator i = uses->begin(), e = uses->end(); i != e; ++i) {
       const VarDecl *vd = i->first;
-      UsesVec *vec = i->second;
+      const UsesMap::mapped_type &V = i->second;
 
-      // Sort the uses by their SourceLocations.  While not strictly
+      UsesVec *vec = V.first;
-      // guaranteed to produce them in line/column order, this will provide
+      bool hasSelfInit = V.second;
-      // a stable ordering.
+
-      std::sort(vec->begin(), vec->end(), SLocSort());
+      // Specially handle the case where we have uses of an uninitialized 
-      
+      // variable, but the root cause is an idiomatic self-init.  We want
-      for (UsesVec::iterator vi = vec->begin(), ve = vec->end(); vi != ve;
+      // to report the diagnostic at the self-init since that is the root cause.
-           ++vi) {
+      if (!vec->empty() && hasSelfInit)
-        if (DiagnoseUninitializedUse(S, vd, vi->first,
+        DiagnoseUninitializedUse(S, vd, vd->getInit()->IgnoreParenCasts(),
-                                      /*isAlwaysUninit=*/vi->second))
+                                 true, /* alwaysReportSelfInit */ true);
-          // Skip further diagnostics for this variable. We try to warn only on
+      else {
-          // the first point at which a variable is used uninitialized.
+        // Sort the uses by their SourceLocations.  While not strictly
-          break;
+        // guaranteed to produce them in line/column order, this will provide
+        // a stable ordering.
+        std::sort(vec->begin(), vec->end(), SLocSort());
+        
+        for (UsesVec::iterator vi = vec->begin(), ve = vec->end(); vi != ve;
+             ++vi) {
+          if (DiagnoseUninitializedUse(S, vd, vi->first,
+                                        /*isAlwaysUninit=*/vi->second))
+            // Skip further diagnostics for this variable. We try to warn only
+            // on the first point at which a variable is used uninitialized.
+            break;
+        }
       }
-
+      
+      // Release the uses vector.
       delete vec;
     }
     delete uses;

clang/test/Sema/uninit-variables.c

@@ -94,10 +94,15 @@ void test14() {
   for (;;) {}
 }
 
-int test15() {
+void test15() {
-  int x = x; // no-warning: signals intended lack of initialization. \
+  int x = x; // no-warning: signals intended lack of initialization.
-             // expected-note{{variable 'x' is declared here}}
+}
-  return x; // expected-warning{{variable 'x' is uninitialized when used here}}
+
+int test15b() {
+  // Warn here with the self-init, since it does result in a use of
+  // an unintialized variable and this is the root cause.
+  int x = x; // expected-warning {{variable 'x' is uninitialized when used within its own initialization}}
+  return x;
 }
 
 // Don't warn in the following example; shows dataflow confluence.