[PATCH] D69360: [NFC] Refactor representation of materialized temporaries

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks, looks good subject to some comment fixes + a tiny change to `RecursiveASTVisitor`.



================
Comment at: clang/include/clang/AST/DeclCXX.h:3055
 
+/// Implicit declartion of a temporary that was materialized by
+/// MaterializeTemporaryExpr
----------------
Typo: declartion -> declaration


================
Comment at: clang/include/clang/AST/DeclCXX.h:3056
+/// Implicit declartion of a temporary that was materialized by
+/// MaterializeTemporaryExpr
+class LifetimeExtendedTemporaryDecl final : public Decl {
----------------
"a MaterializeTemporaryExpr."

We should probably also mention here that this only applies to `MaterializeTemporaryExpr`s that are lifetime-extended.


================
Comment at: clang/include/clang/AST/DeclCXX.h:3098
+
+  /// this isn't necessarily the initializer of the temporary due to the C++98
+  /// delayed materialization rules, but that skipRValueSubobjectAdjustments can
----------------
Start this comment by explaining what the function returns: "Retrieve the expression to which the temporary materialization conversion was applied."

"this" -> "This"


================
Comment at: clang/include/clang/AST/DeclCXX.h:3099
+  /// this isn't necessarily the initializer of the temporary due to the C++98
+  /// delayed materialization rules, but that skipRValueSubobjectAdjustments can
+  /// be used to find said initializer within the subexpression.
----------------
Remove "that" here.


================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1438
 
+DEF_TRAVERSE_DECL(LifetimeExtendedTemporaryDecl, {})
+
----------------
We should traverse the subexpression from here. (If someone directly starts a recursive visitation from the `LifetimeExtendedTemporaryDecl`, they should reach the subexpression.) ...


================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2642-2643
+  if (S->getLifetimeExtendedTemporaryDecl())
+    TRY_TO(TraverseLifetimeExtendedTemporaryDecl(
+        S->getLifetimeExtendedTemporaryDecl()));
+})
----------------
... and we should set `ShouldVisitChildren = false;` here because we already visited the children when visiting the `LifetimeExtendedTemporaryDecl`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69360/new/

https://reviews.llvm.org/D69360