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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
