[cfe-commits] r86061 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclCXX.cpp test/SemaCXX/constructor-initializer.cpp

[John McCall]

Author: rjmccall
Date: Wed Nov  4 17:02:40 2009
New Revision: 86061

URL: http://llvm.org/viewvc/llvm-project?rev=86061&view=rev
Log:
Diagnose using a field to initialize itself.  Patch by Brandon Pearcy!


Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/test/SemaCXX/constructor-initializer.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=86061&r1=86060&r2=86061&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov  4 17:02:40 2009
@@ -518,6 +518,8 @@
   "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<

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=86061&r1=86060&r2=86061&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Nov  4 17:02:40 2009
@@ -971,10 +971,69 @@
                               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();

Modified: cfe/trunk/test/SemaCXX/constructor-initializer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constructor-initializer.cpp?rev=86061&r1=86060&r2=86061&view=diff

==============================================================================
--- cfe/trunk/test/SemaCXX/constructor-initializer.cpp (original)
+++ cfe/trunk/test/SemaCXX/constructor-initializer.cpp Wed Nov  4 17:02:40 2009
@@ -122,3 +122,36 @@
 
   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}}
+};