void added inline comments.
================
Comment at: include/clang/AST/Expr.h:908-912
+ static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+ if (ConstantExpr *CE = dyn_cast<ConstantExpr>(E))
+ return CE;
+ return new (Context) ConstantExpr(E);
+ }
----------------
rsmith wrote:
> If we're creating two `ConstantExpr` wrappers for the same expression, that
> seems like a bug in the caller to me. When does this happen?
I saw a place that was trying to do that, but it may no longer be valid. I can
remove the check for now.
================
Comment at: lib/AST/Expr.cpp:2915-2916
+ case ConstantExprClass: {
+ const Expr *Exp = cast<ConstantExpr>(this)->getSubExpr();
+ return Exp->isConstantInitializer(Ctx, false, Culprit);
+ }
----------------
rsmith wrote:
> Can we just return `true` here? If not, please add a FIXME saying that we
> should be able to.
To be honest, I don't know. Theoretically we should be able to. Let me give it
a try and see how it goes.
================
Comment at: lib/Sema/SemaExpr.cpp:14156-14157
+ if (!CurContext->isFunctionOrMethod())
+ E = ConstantExpr::Create(Context, E);
+
----------------
rsmith wrote:
> I don't understand why `CurContext` matters here. Can you explain the intent
> of this check?
It may have been an artifact of development. I can remove it.
Repository:
rC Clang
https://reviews.llvm.org/D54355
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
