[PATCH] D53921: Compound literals, enums, et al require const expr

Bill Wendling via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 8 15:37:27 PST 2018


void marked 2 inline comments as done.
void added inline comments.


================
Comment at: include/clang/AST/Expr.h:3073-3074
+      e = ice->getSubExpr();
+    else if (ConstantExpr *ce = dyn_cast<ConstantExpr>(e))
+      e = ce->getSubExpr();
+    else
----------------
rsmith wrote:
> Should we skip an arbitrary `FullExpr` here (and in the other `Ignore` functions below)?
Sure. That seems reasonable.


================
Comment at: lib/Sema/SemaType.cpp:2236-2238
+  if (ArraySize && !CurContext->isFunctionOrMethod())
+    // A file-scoped array must have a constant array size.
+    ArraySize = new (Context) ConstantExpr(ArraySize);
----------------
rsmith wrote:
> As noted above, I'd prefer for `VerifyIntegerConstantExpression` to create this. But if we need to do it here, we should do it on the code path that creates a `ConstantArrayType`, not based on whether the array type appears at file scope.
I think my next patch takes care of this (moves it to `VerifyIntegerConstantExpression`). There may be conflicts between the two patches though, so I'd like to move this there in the follow-up patch.


Repository:
  rC Clang

https://reviews.llvm.org/D53921





More information about the cfe-commits mailing list