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