[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