[analyzer][UninitializedObjectChecker] Drop lambda support

[Szelethus]

Summary

After some thinking, I don't think this checker should support lambdas.

Reason 1.: While this is due to the analyzer not being smart enough just yet, the checker doesn't find uninitialized variables that were captured but value, but it does find find them if they were captured by reference. I think this makes little sense -- capturing by reference could be intentional, if the lambda function assigns a value to it, while capturing an undefined variable by value almost never makes sense. This could be fixed, but...

Reason 2.: I don't think lambda misuse should be the responsibility of this checker. It just doesn't make sense, as lambda misuse in not really an uninitialized value problem. It should rather be handled by a standalone lambda checker.

[Szelethus]

[...]lambda misuse in not really an uninitialized value problem.

I guess you can make the argument that it its. Even then, in my opinion this checker is overkill for lambdas.

• If the captured variable has a non-default constructor, UninitializedValueChecker will analyze it,
• if the captured variable has a default constructor, or if Pedantic is disabled and all fields are uninitialized, we probably don't want to warn for them anyways,
• if the captured variable is primitive (BuiltinType or EnumeralType) we can just easily iterate over the captured fields to check whether they are undefined, and make a decision whether to warn for them or not.

My point is, a standalone cplusplus.LambdaMisuse checker would be a lot more fitting to handle these cases.

But I'd love to hear your thoughts on this one! This functionality could totally be squeezed into the checker (with better warning messages), if we were to come to the conclusion that it should be.

[Szelethus]

Turns out I was very wrong on this one. Lambdas should be ignored as data members, but not in general.

Sorry about the spam, I think I should've sit longer on this one. I'll submit another patch tomorrow that will correctly explain my current way of thinking on this issue.

lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp

private:
 
   /// This method returns true if the value of a primitive object is
   /// uninitialized.
   bool isPrimitiveUninit(const SVal &V);
 
   // Note that we don't have a method for arrays -- the elements of an array are
   // often left uninitialized intentionally even when it is of a C++ record
   // type, so we'll assume that an array is always initialized.
+  //
+  // We'll also ignore lambdas, as lambda data members may contain uninitialized
+  // captured variables, but checking for them (and lambda misuse in general)
+  // shouldn't be the resposibility of this checker.
+  //
   // TODO: Add a support for nonloc::LocAsInteger.
 };
 
 // Utility function declarations.
 
 /// Returns the object that was constructed by CtorDecl, or None if that isn't
 /// possible.
 Optional<nonloc::LazyCompoundVal>
for (const FieldDecl *I : RD->fields()) {
     QualType T = I->getType();
 
     // If LocalChain already contains FR, then we encountered a cyclic
     // reference. In this case, region FR is already under checking at an
     // earlier node in the directed tree.
     if (LocalChain.contains(FR))
       return false;
 
+    if (const auto *CXXRD = T->getAsCXXRecordDecl()) {
+      if (CXXRD->isLambda()) {
+        IsAnyFieldInitialized = true;
+        continue;
+      }
+    }
+
     if (T->isStructureOrClassType()) {
       if (isNonUnionUninit(FR, {LocalChain, FR}))
         ContainsUninitField = true;
       continue;
     }
 
     if (T->isUnionType()) {
       if (isUnionUninit(FR)) {
if (Optional<nonloc::LazyCompoundVal> RecordV =
     // similar code snippets.
     if (R->getSymbolicBase()) {
       IsAnyFieldInitialized = true;
       return false;
     }
 
     const QualType T = R->getValueType();
 
+    if (const auto *CXXRD = T->getAsCXXRecordDecl()) {
+      if (CXXRD->isLambda()) {
+        IsAnyFieldInitialized = true;
+        return false;
+      }
+    }
+
     if (T->isStructureOrClassType())
       return isNonUnionUninit(R, {LocalChain, FR});
 
     if (T->isUnionType()) {
       if (isUnionUninit(R)) {
         return addFieldToUninits({LocalChain, FR, /*IsDereferenced*/ true});
       } else {
         IsAnyFieldInitialized = true;

 //   struct MostDerived : D1, D2 {
 //     MostDerived() {}
 //   }
 //
 // A call to MostDerived::MostDerived() will cause two notes that say
 // "uninitialized field 'this->x'", but we can't refer to 'x' directly,
 // we need an explicit namespace resolution whether the uninit field was
 // 'D1::x' or 'D2::x'.
-//
-// TODO: If a field in the fieldchain is a captured lambda parameter, this
-// function constructs an empty string for it:
-//
-//   template <class Callable> struct A {
-//     Callable c;
-//     A(const Callable &c, int) : c(c) {}
-//   };
-//
-//   int b; // say that this isn't zero initialized
-//   auto alwaysTrue = [&b](int a) { return true; };
-//
-// A call with these parameters: A<decltype(alwaysTrue)>::A(alwaysTrue, int())
-// will emit a note with the message "uninitialized field: 'this->c.'". If
-// possible, the lambda parameter name should be retrieved or be replaced with a
-// "<lambda parameter>" or something similar.
 void FieldChainInfo::print(llvm::raw_ostream &Out) const {
   if (Chain.isEmpty())
     return;
 
   const llvm::ImmutableListImpl<const FieldRegion *> *L =
       Chain.getInternalPointer();
   printTail(Out, L->getTail());
   Out << L->getHead()->getDecl()->getNameAsString();

test/Analysis/cxx-uninitialized-object.cpp

 void fMemsetTest2() {
   MemsetTest2();
 }
 
 //===----------------------------------------------------------------------===//
 // Lambda tests.
 //===----------------------------------------------------------------------===//
 
+// The checker no longer checks uninitialized captured lambda variables, these
+// test are mainly no-crash test.
+
 template <class Callable>
 struct LambdaTest1 {
   Callable functor;
 
   LambdaTest1(const Callable &functor, int) : functor(functor) {
     // All good!
   }
 };
 
 void fLambdaTest1() {
   auto isEven = [](int a) { return a % 2 == 0; };
   LambdaTest1<decltype(isEven)>(isEven, int());
 }
 
 #ifdef PEDANTIC
 template <class Callable>
 struct LambdaTest2 {
   Callable functor;
 
-  LambdaTest2(const Callable &functor, int) : functor(functor) {} // expected-warning{{1 uninitialized field}}
+  LambdaTest2(const Callable &functor, int) : functor(functor) {}
 };
 
 void fLambdaTest2() {
   int b;
-  auto equals = [&b](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.'}}
+  auto equals = [&b](int a) { return a == b; };
   LambdaTest2<decltype(equals)>(equals, int());
 }
 #else
 template <class Callable>
 struct LambdaTest2 {
   Callable functor;
 
   LambdaTest2(const Callable &functor, int) : functor(functor) {}
 };
 
 void fLambdaTest2() {
   int b;
   auto equals = [&b](int a) { return a == b; };
   LambdaTest2<decltype(equals)>(equals, int());
 }
 #endif //PEDANTIC
 
 #ifdef PEDANTIC
 namespace LT3Detail {
 
 struct RecordType {
-  int x; // expected-note{{uninitialized field 'this->functor..x'}}
-  int y; // expected-note{{uninitialized field 'this->functor..y'}}
+  int x;
+  int y;
 };
 
 } // namespace LT3Detail
 template <class Callable>
 struct LambdaTest3 {
   Callable functor;
 
-  LambdaTest3(const Callable &functor, int) : functor(functor) {} // expected-warning{{2 uninitialized fields}}
+  LambdaTest3(const Callable &functor, int) : functor(functor) {}
 };
 
 void fLambdaTest3() {
   LT3Detail::RecordType rec1;
   auto equals = [&rec1](LT3Detail::RecordType rec2) {
     return rec1.x == rec2.x;
   };
   LambdaTest3<decltype(equals)>(equals, int());