[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.

Looks good overall, but some nits inline.



================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:682
 
+StringRef getVariableName(const FieldDecl *Field) {
+  const auto *CXXParent = dyn_cast<CXXRecordDecl>(Field->getParent());
----------------
Can we have a comment explaining the semantics? From my understanding, you find a corresponding capture using a source location, and get a name from there.


================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:685
+
+  if (CXXParent && CXXParent->isLambda()) {
+    CXXRecordDecl::capture_const_iterator CapturedVar =
----------------
CXXParent is guaranteed to be non-null at this stage, otherwise dyn_cast fails


================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:687
+    CXXRecordDecl::capture_const_iterator CapturedVar =
+        std::find_if(CXXParent->captures_begin(), CXXParent->captures_end(),
+                     [&Field](const LambdaCapture &L) {
----------------
Could we just use a for-each here? Sorry for being opinionated, I'm pretty sure it would be both shorter and more readable.


================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:689
+                     [&Field](const LambdaCapture &L) {
+                       return Field->getLocation().getRawEncoding() ==
+                              L.getLocation().getRawEncoding();
----------------
That's exactly the semantics of the equality operator on source locations, so why not `Field->getLocation() == L.getLocation()` ?


Repository:
  rC Clang

https://reviews.llvm.org/D48291