[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/include/clang/AST/ExprCXX.h:1274
+      *getTrailingObjects<Expr *>() = RewrittenExpr;
     setDependence(computeDependence(this));
   }
----------------
aaron.ballman wrote:
> Do we need to update `computeDependence()` to account for the rewritten `Expr *`?
Looking at the code again, `computeDependence` do use the rewritten expr if there is one already (contracticting what I previously stated offline)


================
Comment at: clang/include/clang/Sema/Sema.h:9654-9655
+        return true;
+      if (Ctx.isConstantEvaluated() || Ctx.isImmediateFunctionContext() ||
+          Ctx.isUnevaluated())
+        return false;
----------------
aaron.ballman wrote:
> 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.
`isNotPotentiallyEvaluatedContext`? Something like that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554