[PATCH] D81154: [AST][RecoveryExpr] Populate the dependence bits from CompoundStmt result expr to StmtExpr.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/AST/ComputeDependence.cpp:131
 ExprDependence clang::computeDependence(StmtExpr *E, unsigned TemplateDepth) {
   // FIXME: why is unexpanded-pack not propagated?
   auto D = toExprDependence(E->getType()->getDependence()) &
----------------
I think I know the answer now - you can't expand a param pack over stmtexpr boundaries. Could repace this comment...


================
Comment at: clang/lib/AST/ComputeDependence.cpp:132
   // FIXME: why is unexpanded-pack not propagated?
   auto D = toExprDependence(E->getType()->getDependence()) &
            ~ExprDependence::UnexpandedPack;
----------------
I wonder if this is still needed - how can we get a type for the stmtexpr unless we have a result expr providing the type?


================
Comment at: clang/lib/AST/ComputeDependence.cpp:134
            ~ExprDependence::UnexpandedPack;
   // Note: we treat a statement-expression in a dependent context as always
   // being value- and instantiation-dependent. This matches the behavior of
----------------
Nit: i'd probably leave this as the last clause as it's the weird edge case.


================
Comment at: clang/lib/AST/ComputeDependence.cpp:140
+
+  // Populate dependence bits from the expr that consider to be the result
+  // of the compound statements.
----------------
Maybe just "propagate dependence of the result"? The compound statement seems like a red herring.


================
Comment at: clang/lib/AST/ComputeDependence.cpp:145
+    if (const Expr *ResultExpr = CompoundExprResult->getExprStmt())
+      D |= ResultExpr->getDependence();
   return D;
----------------
this should avoid propagating the pack bit as above (or factor it out and mask it off at the end to avoid writing it twice)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81154