CheckConstantExpression issue


I encountered the following issue in lib/AST/ExprConstant.cpp:

In function CheckConstantExpr there is a code:

  if (Value.isUnion() && Value.getUnionField()) {
    return CheckConstantExpression(Info, DiagLoc,
  if (Value.isStruct()) {
    RecordDecl *RD = Type->castAs<RecordType>()->getDecl(); //// <<<<<< The problematic line
    if (const CXXRecordDecl *CD = dyn_cast<CXXRecordDecl>(RD)) {
      unsigned BaseIndex = 0;
      for (CXXRecordDecl::base_class_const_iterator I = CD->bases_begin(),
             End = CD->bases_end(); I != End; ++I, ++BaseIndex) {
        if (!CheckConstantExpression(Info, DiagLoc, I->getType(),
          return false;

The problematic line causes a crash because Type cannot be cast to RecordType.

This looks like an issue in clang to me, but the question is what's wrong: Value.isStruct() returning true for constant of template parameter type (see below) or absence of check for Type to be RecordType.
I will definitely impelement workaround in my code, but I would also like to investingate and fix this issue in clang.


----- useful info ----

The code which triggering this behavior looks like the following:
    template<class T>
    void DoTestMax() {
        const T max = Max();

I am trying to access and print constant initializer for 'max' variable under following checks:
            } else if (auto* varDecl = llvm::dyn_cast<clang::VarDecl>(decl)) {
                if (!clang::isa<clang::ParmVarDecl>(varDecl) && (varDecl->isConstexpr() || varDecl->getType().isConstQualified())) {
                    const clang::Expr* init = varDecl->getAnyInitializer();
                    if (init != nullptr &&
                        !init->getType().isNull() &&
                        !init->isValueDependent() &&
                        varDecl->ensureEvaluatedStmt() != nullptr &&
                        varDecl->ensureEvaluatedStmt()->Value != nullptr)
                        const clang::APValue* val = varDecl->evaluateValue(); /// <<< Problem is here

The relevant stack for debug clang (trunk) looks like below:
#3 0x00007ffff6bb2ca2 in __GI___assert_fail (assertion=0x2661ae0 "isa<X>(Val) && \"cast<Ty>() argument of incompatible type!\"", file=0x2661aa0 "/home/spreis/LLVM/latest/llvm/include/llvm/Support/Casting.h", line=241,
    function=0x2663de0 <std::enable_if<!llvm::is_simple_type<clang::QualType>::value, llvm::cast_retty<clang::RecordType, clang::QualType const>::ret_type>::type llvm::cast<clang::RecordType, clang::QualType>(clang::QualType const&)::__PRETTY_FUNCTION__> "typename std::enable_if<(! llvm::is_simple_type<Y>::value), typename llvm::cast_retty<X, const Y>::ret_type>::type llvm::cast(const Y&) [with X = clang::RecordType; Y = clang::QualType; typename std::"...)
    at assert.c:101
#4 0x00000000010cf75e in llvm::cast<clang::RecordType, clang::QualType> (Val=...)
    at /home/spreis/LLVM/latest/llvm/include/llvm/Support/Casting.h:241
#5 0x00000000010ce4fa in clang::Type::castAs<clang::RecordType> (this=0x554a880)
    at /home/spreis/LLVM/latest/llvm/tools/clang/include/clang/AST/TypeNodes.def:122
#6 0x0000000001c0b810 in CheckConstantExpression (Info=..., DiagLoc=..., Type=..., Value=...)
    at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/ExprConstant.cpp:1737
#7 0x0000000001c2f1b0 in clang::Expr::EvaluateAsInitializer (this=0x554ab50, Value=..., Ctx=..., VD=0x554a9d8, Notes=...)
    at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/ExprConstant.cpp:10079
#8 0x0000000001b923fa in clang::VarDecl::evaluateValue (this=0x554a9d8, Notes=...)
    at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/Decl.cpp:2172
#9 0x0000000001b9229c in clang::VarDecl::evaluateValue (this=0x554a9d8)
    at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/Decl.cpp:2146

The var:
(gdb) frame 9
2146 return evaluateValue(Notes);
(gdb) call this->dump()
VarDecl 0x554a9d8 <..snip..snip> referenced max 'const T' cinit
`-CallExpr 0x554ab50 <col:23, col:27> '::NPrivate::TMax':'struct NPrivate::TMax'
  `-ImplicitCastExpr 0x554ab38 <col:23> '::NPrivate::TMax (*)(void) noexcept' <FunctionToPointerDecay>
    `-DeclRefExpr 0x554aab8 <col:23> '::NPrivate::TMax (void) noexcept' lvalue Function 0x3f82c28 'Max' '::NPrivate::TMax (void) noexcept'

The type:
(gdb) frame 6
1737 RecordDecl *RD = Type->castAs<RecordType>()->getDecl();
(gdb) call Type.dump()
QualType 0x554a881 'const T' const
`-TemplateTypeParmType 0x554a880 'T' dependent depth 0 index 0
  `-TemplateTypeParm 0x554a840 'T'

It looks like you’re trying to evaluate a variable with a dependent type. I think that evaluateValue function is not intended for such variables. I think that it might be a good idea to put an assertion into that function that checks if the variables or its type is dependent, to complement the assertion for the dependent initializer in that function.


thank you for you prompt reply.

I did exactly this in my code, but I am not sure asserts are enough. I already added checks to avoid some issues like this:

                    varDecl\-&gt;ensureEvaluatedStmt\(\) \!= nullptr &amp;&amp;
                    varDecl\-&gt;ensureEvaluatedStmt\(\)\-&gt;Value \!= nullptr\)
                    \!init\-&gt;isValueDependent\(\) &amp;&amp;

Against following code in Decl.cpp:

APValue *VarDecl::evaluateValue(
    SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
  EvaluatedStmt *Eval = ensureEvaluatedStmt();
... // .... skipped comment ...
  if (Eval->WasEvaluated) /// <<<<< Here non-NULL pointer is blindly assumed
    return Eval->Evaluated.isUninit() ? nullptr : &Eval->Evaluated;

  const auto *Init = cast<Expr>(Eval->Value);
  assert(!Init->isValueDependent()); /// <<<< Here is just assert which only fires in debug mode

  if (Eval->IsEvaluating) {

I think at least asserts should be added to VarDecl::evaluateValue() to ensure Eval is not NULL and the type of 'this' VarDecl is not DependentType. But in this case all prerequisites for evaluateValue are better to be somehow documented.

Now for the simple varDecl->evaluateValue() we have at least (+ maybe something else?)
                       (varDecl->getAnyInitializer() != nullptr) /// This
                        !init->getType().isNull() && /// and this
                        !init->isValueDependent() &&
                        !init->isTypeDependent() &&
                        !varDecl->getType().isNull() && /// and this --- seem reasonable to have outside the call, but I am not that sure about all others
                        !varDecl->getType()->isDependentType() &&
                        varDecl->ensureEvaluatedStmt() != nullptr &&
                        varDecl->ensureEvaluatedStmt()->Value != nullptr)

Alternatively let the function return NULL in all unsupported cases basically placing all required checks insde the function. This function already returns NULL on various occasions, so these additional checks seem to me natutal fit to the function's logic.

I am ready to prepare patch for review based on agreed decision.


Have you tried to check if varDecl is in a dependent context before calling the CheckConstantExpression?


Thank you,

Thank you, Mikhail.

Yes, this definitely works. My question is not about making my code working: I already fixed it. My question is what checks should belong to callee (VarDecl::evaluateValue()) opposed to caller (my code) and what actions are to be taken upon these checks.
I believe that something broken in VarDecl::evaluateValue() logic:
(a) It already encorporates quite a few checks and returns nullptr on a quite a few occasions
(b) On some other occasions (pretty similar IMO to ones above) it only has asserts
(c) Callee lacks some checks even in form of assetions making assumptions those are not that trivial and basic.

So I started this thread not in order to make my code working, but in attempt to seek a way to improve clang usability around VarDecl::evaluateValue() functionality. So maybe it is worth adding

+ if (getDeclContext()->isDependentContext()) {
+ return nullptr;
+ }

to VarDecl::evaluateValue() implementation (although this may be too broad check and varDecl->getType()->isDependentType() is more precise and appropriate).

I would also added the following checks:

+ if (!ensureEvaluatedStmt() || !ensureEvaluatedStmt()->Value) {
+ return nullptr
+ }


const auto *Init = cast<Expr>(Eval->Value);
+ if (!Init || Init->getType().isNull() || Init->isValueDependent() || Init->isTypeDependent()) {
+ return nullptr;
+ }

Thank you,