Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, dkrupp.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, 
whisperity.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D48318

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
  test/Analysis/cxx-uninitialized-object.cpp

Index: test/Analysis/cxx-uninitialized-object.cpp
===================================================================
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -736,6 +736,9 @@
 // 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;
@@ -755,12 +758,12 @@
 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
@@ -782,16 +785,16 @@
 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() {
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -202,6 +202,11 @@
   // 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.
 };
 
@@ -373,6 +378,13 @@
     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;
@@ -518,6 +530,13 @@
 
     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});
 
@@ -610,22 +629,6 @@
 // "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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to