[PATCH] D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts.

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.

LGTM, subject to minor nits



================
Comment at: include/clang/Analysis/CFG.h:172
 /// by value. This, like constructor, requires a construction context, which
-/// will always be that of a temporary object - usually consumed by an elidable
-/// constructor. For such value-typed calls the ReturnedValueConstructionContext
-/// of their return value is naturally complemented by the
-/// TemporaryObjectConstructionContext at the call site (here). In C such
+/// will usually be that of a temporary object - usually consumed by an elidable
+/// constructor. In C++17 it may also be a construction for a caller function's
----------------
s/usually/pre-c++17?


================
Comment at: include/clang/Analysis/ConstructionContext.h:157
+public:
+  explicit SimpleVariableConstructionContext(const DeclStmt *DS)
+      : VariableConstructionContext(ConstructionContext::SimpleVariableKind,
----------------
👍 for explicit constructors


================
Comment at: lib/Analysis/ConstructionContext.cpp:74
+          return new (CC) TemporaryObjectConstructionContext(BTE, MTE);
+        } else {
+          // C++17 *requires* elision of the constructor at the return site
----------------
`else` not needed, as previous statement returns


================
Comment at: lib/Analysis/ConstructionContext.cpp:87
+                CXX17ElidedCopyReturnedValueConstructionContext(RS, BTE);
+          } else {
+            auto *DS = cast<DeclStmt>(ParentLayer->getTriggerStmt());
----------------
this `else` also seems redundant


================
Comment at: lib/Analysis/ConstructionContext.cpp:98
+        }
+      } else {
+        // A temporary object that doesn't require materialization.
----------------
...and this one as well


Repository:
  rC Clang

https://reviews.llvm.org/D44597