From: John McCall <rjmccall@apple.com>
Date: Wed, 4 Nov 2009 23:02:40 +0000 (+0000)
Subject: Diagnose using a field to initialize itself.  Patch by Brandon Pearcy!
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b41900410fae8cba4bc02db2e3e44142e7f4c625;p=clang

Diagnose using a field to initialize itself.  Patch by Brandon Pearcy!



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@86061 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 368496975a..1ce7dbc949 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -518,6 +518,8 @@ def err_init_reference_member_uninitialized : Error<
   "reference member of type %0 uninitialized">;
 def note_uninit_reference_member : Note<
   "uninitialized reference member is here">;
+def warn_field_is_uninit : Warning<"field is uninitialized when used here">,
+  InGroup<DiagGroup<"uninitialized">>;
 
 // C++0x decltype
 def err_cannot_determine_declared_type_of_overloaded_function : Error<
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index d1f9ad960f..fdbd554e03 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -971,10 +971,69 @@ Sema::ActOnMemInitializer(DeclPtrTy ConstructorD,
                               RParenLoc, ClassDecl);
 }
 
+/// Checks an initializer expression for use of uninitialized fields, such as
+/// containing the field that is being initialized. Returns true if there is an
+/// uninitialized field was used an updates the SourceLocation parameter; false
+/// otherwise.
+static bool InitExprContainsUninitializedFields(const Stmt* S,
+                                                const FieldDecl* LhsField,
+                                                SourceLocation* L) {
+  const MemberExpr* ME = dyn_cast<MemberExpr>(S);
+  if (ME) {
+    const NamedDecl* RhsField = ME->getMemberDecl();
+    if (RhsField == LhsField) {
+      // Initializing a field with itself. Throw a warning.
+      // But wait; there are exceptions!
+      // Exception #1:  The field may not belong to this record.
+      // e.g. Foo(const Foo& rhs) : A(rhs.A) {}
+      const Expr* base = ME->getBase();
+      if (base != NULL && !isa<CXXThisExpr>(base->IgnoreParenCasts())) {
+        // Even though the field matches, it does not belong to this record.
+        return false;
+      }
+      // None of the exceptions triggered; return true to indicate an
+      // uninitialized field was used.
+      *L = ME->getMemberLoc();
+      return true;
+    }
+  }
+  bool found = false;
+  for (Stmt::const_child_iterator it = S->child_begin();
+       it != S->child_end() && found == false;
+       ++it) {
+    if (isa<CallExpr>(S)) {
+      // Do not descend into function calls or constructors, as the use
+      // of an uninitialized field may be valid. One would have to inspect
+      // the contents of the function/ctor to determine if it is safe or not.
+      // i.e. Pass-by-value is never safe, but pass-by-reference and pointers
+      // may be safe, depending on what the function/ctor does.
+      continue;
+    }
+    found = InitExprContainsUninitializedFields(*it, LhsField, L);
+  }
+  return found;
+}
+
 Sema::MemInitResult
 Sema::BuildMemberInitializer(FieldDecl *Member, Expr **Args,
                              unsigned NumArgs, SourceLocation IdLoc,
                              SourceLocation RParenLoc) {
+  // Diagnose value-uses of fields to initialize themselves, e.g.
+  //   foo(foo)
+  // where foo is not also a parameter to the constructor.
+  for (unsigned i = 0; i < NumArgs; ++i) {
+    SourceLocation L;
+    if (InitExprContainsUninitializedFields(Args[i], Member, &L)) {
+      // FIXME: Return true in the case when other fields are used before being
+      // uninitialized. For example, let this field be the i'th field. When
+      // initializing the i'th field, throw a warning if any of the >= i'th
+      // fields are used, as they are not yet initialized.
+      // Right now we are only handling the case where the i'th field uses
+      // itself in its initializer.
+      Diag(L, diag::warn_field_is_uninit);
+    }
+  }
+
   bool HasDependentArg = false;
   for (unsigned i = 0; i < NumArgs; i++)
     HasDependentArg |= Args[i]->isTypeDependent();
diff --git a/test/SemaCXX/constructor-initializer.cpp b/test/SemaCXX/constructor-initializer.cpp
index b86a27d66d..20cf35b293 100644
--- a/test/SemaCXX/constructor-initializer.cpp
+++ b/test/SemaCXX/constructor-initializer.cpp
@@ -122,3 +122,36 @@ struct Q {
 
   float *pf;
 };
+
+// A silly class used to demonstrate field-is-uninitialized in constructors with
+// multiple params.
+class TwoInOne { TwoInOne(TwoInOne a, TwoInOne b) {} };
+class InitializeUsingSelfTest {
+  bool A;
+  char* B;
+  int C;
+  TwoInOne D;
+  InitializeUsingSelfTest(int E)
+      : A(A),  // expected-warning {{field is uninitialized when used here}}
+        B((((B)))),  // expected-warning {{field is uninitialized when used here}}
+        C(A && InitializeUsingSelfTest::C),  // expected-warning {{field is uninitialized when used here}}
+        D(D,  // expected-warning {{field is uninitialized when used here}}
+          D) {}  // expected-warning {{field is uninitialized when used here}}
+};
+
+int IntWrapper(int i) { return 0; };
+class InitializeUsingSelfExceptions {
+  int A;
+  int B;
+  InitializeUsingSelfExceptions(int B)
+      : A(IntWrapper(A)),  // Due to a conservative implementation, we do not report warnings inside function/ctor calls even though it is possible to do so.
+        B(B) {}  // Not a warning; B is a local variable.
+};
+
+class CopyConstructorTest {
+  bool A, B, C;
+  CopyConstructorTest(const CopyConstructorTest& rhs)
+      : A(rhs.A),
+        B(B),  // expected-warning {{field is uninitialized when used here}}
+        C(rhs.C || C) { }  // expected-warning {{field is uninitialized when used here}}
+};