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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
