[PATCH] D136554: Implement CWG2631
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 28 06:05:48 PST 2022
aaron.ballman added reviewers: usaxena95, erichkeane, hubert.reinterpretcast.
aaron.ballman added a comment.
The changes generally LGTM, but I'd appreciate a second set of eyes on the changes just to double-check we're implementing the resolution properly.
================
Comment at: clang/include/clang/AST/ExprCXX.h:1274
+ *getTrailingObjects<Expr *>() = RewrittenExpr;
setDependence(computeDependence(this));
}
----------------
Do we need to update `computeDependence()` to account for the rewritten `Expr *`?
================
Comment at: clang/include/clang/Sema/Sema.h:1344
+ assert(Decl && Context && "invalid initialization context");
+ };
+
----------------
It took me a moment to realize this wasn't the end of the structure declaration. :-D
================
Comment at: clang/include/clang/Sema/Sema.h:9654-9655
+ return true;
+ if (Ctx.isConstantEvaluated() || Ctx.isImmediateFunctionContext() ||
+ Ctx.isUnevaluated())
+ return false;
----------------
We repeat this pattern four times in this patch; perhaps we should make a helper function for this predicate? Not certain of the best name for it, but removing the duplication might not be a bad idea.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136554/new/
https://reviews.llvm.org/D136554
More information about the cfe-commits
mailing list