rsmith added inline comments.
================
Comment at: lib/AST/ExprConstant.cpp:5266-5268
+ bool VisitConstantExpr(const ConstantExpr *E) {
+ return Visit(E->getSubExpr());
+ }
----------------
This shouldn't be necessary because you changed the CRTP base class
`ExprEvaluatorBase` to do this already.
================
Comment at: lib/AST/ExprConstant.cpp:5744-5746
+ bool VisitConstantExpr(const ConstantExpr *E) {
+ return evaluateLValue(E->getSubExpr(), Result);
+ }
----------------
Likewise here.
================
Comment at: lib/Sema/SemaDecl.cpp:16086-16094
CheckConvertedConstantExpression(Val, EltTy, EnumVal,
CCEK_Enumerator);
if (Converted.isInvalid())
Val = nullptr;
else
Val = Converted.get();
} else if (!Val->isValueDependent() &&
----------------
I think it would make sense to create the `ConstantExpr` wrapper node in
`CheckConvertedConstantExpr` / `VerifyIntegerConstantExpr`. (That's also where
it'd make sense to cache the evaluated value on the wrapper node once we start
doing so.)
================
Comment at: lib/Sema/SemaExpr.cpp:5752
!literalType->isDependentType()) // C99 6.5.2.5p3
if (CheckForConstantInitializer(LiteralExpr, literalType))
return ExprError();
----------------
Can we create the `ConstantExpr` in here instead (assuming we need it at all)?
(The reason it's not clear to me that we need it at all is that file-scope
compound literals can only appear in contexts where a constant expression is
required *anyway* -- be they in array bounds, enumerators, or the initializer
of a global variable.)
================
Comment at: lib/Sema/SemaExpr.cpp:5789
+ VK, LiteralExpr, isFileScope);
+ return MaybeBindToTemporary(new (Context) ConstantExpr(CLE));
}
----------------
(This approach is at least not entirely correct: compound literals are only
required to have a constant initializer at file scope.)
================
Comment at: lib/Sema/SemaType.cpp:2181
!T->isConstantSizeType()) ||
isArraySizeVLA(*this, ArraySize, ConstVal)) {
// Even in C++11, don't allow contextual conversions in the array bound
----------------
This call calls `VerifyIntegerConstantExpression`; I think we should be
creating the `ConstantExpr` node within there when appropriate.
Repository:
rC Clang
https://reviews.llvm.org/D53921
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
