This is an archive of the discontinued LLVM Phabricator instance.

[clang][AST] TextNodeDumper should not evaluate the initializer of constexpr variable declaration when it has a dependent type
ClosedPublic

Authored by hazohelet on May 20 2023, 9:53 AM.

Details

Summary

TextNodeDumper enabed through -ast-dump flag should not evlauate the initializer when it visits a constexpr VarDecl node if it has a dependent type.

I found a crashing case fixed by this change and added it as a test case.
template <typename T> constexpr T call_init(0);
Link: https://godbolt.org/z/3bG9Pjj5E

This is a fix for the regression caused by D146358

Diff Detail

Event Timeline

hazohelet created this revision.May 20 2023, 9:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2023, 9:53 AM
hazohelet requested review of this revision.May 20 2023, 9:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2023, 9:53 AM

Can you briefly explain why the FieldDecl we see is null in that case?

Can you briefly explain why the FieldDecl we see is null in that case?

Consider template<typename T> constexpr T foo{};

evaluateValue here leads to a call of EvaluateAsInitializer. It calls CheckConstantExpression, which leads to the call of CheckEvaluationResult with null FieldDecl.
The APValue-typed Value argument passed to CheckConstantExpression should hasValue bacause otherwise it would cause assertion failure we saw before.
This Value object is calculated at Evaluate function called from EvaluateAsInitializer.
The evaluated Expr object is InitListExpr that has type void, so the branch for void-typed expression(https://github.com/llvm/llvm-project/blob/8b1727f8d9104df5ced4bdfdc065dea85ff84baf/clang/lib/AST/ExprConstant.cpp#L15041-L15046) is fired.
In this branch, the Result argument is left untouched, which leads to CheckEvaluationResult receiving Value object whose hasValue evaluates to false.

BTW, I noticed that when the type of the declaration is NOT a dependent type (e.g. template <typename T> constexpr int foo{42} ), we need to evaluate the initializer and dump the value. I'll update the patch and retitle it.

hazohelet updated this revision to Diff 524095.May 21 2023, 4:47 AM
hazohelet retitled this revision from [clang][AST] TextNodeDumper should not evaluate the initializer of constexpr variable template definition to [clang][AST] TextNodeDumper should not evaluate the initializer of constexpr variable declaration when it has a dependent type.
hazohelet edited the summary of this revision. (Show Details)
  • Evaluate the initializer if the type of the declaration is not a dependent type
  • Add a relevant test
  • Update release note for this change
erichkeane accepted this revision.May 22 2023, 6:22 AM
This revision is now accepted and ready to land.May 22 2023, 6:22 AM