[PATCH] D48608: [CFG] [analyzer] Add construction contexts for C++ objects returned from Objective-C messages.

[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.

Minor nits: would like some more comments.



================
Comment at: include/clang/Analysis/CFG.h:185
+    assert(isa<CallExpr>(E) || isa<ObjCMessageExpr>(E));
+    return !E->isGLValue() &&
+           E->getType().getCanonicalType()->getAsCXXRecordDecl();
----------------
Can we have a comment on why are filtering out GLValues here?


================
Comment at: lib/Analysis/CFG.cpp:795
+
+    if (BuildOpts.AddRichCXXConstructors) {
+      if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(ME)) {
----------------
It's tempting to factor this tower of if's into a function returning `bool`.
Then an appropriate name could be used to indicate semantics, which I'm currently not sure about.


================
Comment at: lib/Analysis/CFG.cpp:799
+                ConstructionContextMap.lookup(ME)) {
+          cleanupConstructionContext(ME);
+          if (const auto *CC = ConstructionContext::createFromLayers(
----------------
Do we want to cleanup construction context here even if the condition below is not met?


Repository:
  rC Clang

https://reviews.llvm.org/D48608