[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. > It makes sense to me that const int foo[] = [ ...massive list...]; would take > long to validate the entire initializer as all constant expressions The expensive part we're currently avoiding by bailing out of the constant evaluator (the code D76169

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers added a comment. So, @rsmith are you ok with a patch like https://reviews.llvm.org/D76169 that removes those fixmes? It makes sense to me that `const int foo[] = [ ...massive list...];` would take long to validate the entire initializer as all constant expressions, but I'm

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

tomhughes added a comment. I'm hitting this in the codebase that I'm working on as well. What are the next steps (besides reformatting)? It's not completely clear to me from the comments. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) +return Visit(V->getInit(), V->getType()); +return nullptr; rsmith wrote: > efriedma wrote: > > rsmith wrote: > > > nickdesaulniers wrote:

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

rsmith added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) +return Visit(V->getInit(), V->getType()); +return nullptr; efriedma wrote: > rsmith wrote: > > nickdesaulniers wrote: > > > efriedma wrote:

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) +return Visit(V->getInit(), V->getType()); +return nullptr; rsmith wrote: > nickdesaulniers wrote: > > efriedma wrote: > > > You need to be

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

rsmith added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) +return Visit(V->getInit(), V->getType()); +return nullptr; nickdesaulniers wrote: > efriedma wrote: > > You need to be more careful here; we

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. > Isn't that what my patch is doing? (Codegen walking the AST/InitListExpr, > generating Constants)? Yes. The issue is we we don't really want to duplicate the logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers updated this revision to Diff 250319. nickdesaulniers added a comment. - add support for compile time arrays Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76096/new/ https://reviews.llvm.org/D76096 Files: clang/lib/AST/Expr.cpp

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers added a comment. In D76096#1922239 , @rsmith wrote: > it's substantially more efficient for CodeGen to walk the AST representation > (the `InitListExpr`) and directly generate an IR constant than it is to > create an `APValue`

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. > Removing those two LangOpt checks isn't enough for the test cases to run. Removing those two checks should unblock const-eval for structs in general. I wasn't thinking about whether there something else for global variables specifically, though. It looks like

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

rsmith added a comment. In D76096#1921842 , @nickdesaulniers wrote: > > The performance implications of deleting those lines is the complicated > > part. > > Where does compile time performance suffer from this? I guess if we have > massive array

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers added a comment. In D76096#1920685 , @efriedma wrote: > I think the code that disables constant evaluation for C is just >

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers added inline comments. Comment at: clang/lib/AST/Expr.cpp:3164 + const QualType = cast(this)->getDecl()->getType(); + if (QT->isStructureType() && QT.isConstQualified()) +return true; efriedma wrote: > efriedma wrote: > >

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers planned changes to this revision. nickdesaulniers added a comment. > The performance implications of deleting those lines is the complicated part. Where does compile time performance suffer from this? I guess if we have massive array initializers, or large struct definitions, or

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. I think the code that disables constant evaluation for C is just https://github.com/llvm/llvm-project/blob/dcaf13a4048df3dad55f1a28cde7cefc99ccc057/clang/lib/AST/ExprConstant.cpp#L13918 and

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added inline comments. Comment at: clang/lib/AST/Expr.cpp:3164 + const QualType = cast(this)->getDecl()->getType(); + if (QT->isStructureType() && QT.isConstQualified()) +return true; nickdesaulniers wrote: > efriedma wrote: > >

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers marked an inline comment as done and an inline comment as not done. nickdesaulniers added a comment. In D76096#1920477 , @efriedma wrote: > But that's probably a larger project than you really want to mess with. Maybe with some

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. I would like to kill off all the code you're modifying, and let ExprConstant be the final arbiter of whether a constant is in fact constant. But that's probably a larger project than you really want to mess with. The big missing piece of that is that we currently

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/lib/AST/Expr.cpp:3164 + const QualType = cast(this)->getDecl()->getType(); + if (QT->isStructureType() && QT.isConstQualified()) +return true;

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers updated this revision to Diff 250046. nickdesaulniers added a comment. - add 2 missing CHECKs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76096/new/ https://reviews.llvm.org/D76096 Files: clang/lib/AST/Expr.cpp

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers marked an inline comment as not done. nickdesaulniers added inline comments. Comment at: clang/lib/AST/Expr.cpp:3164 + const QualType = cast(this)->getDecl()->getType(); + if (QT->isStructureType() && QT.isConstQualified()) +return true;

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

nickdesaulniers created this revision. nickdesaulniers added reviewers: eli.friedman, aaron.ballman, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. nickdesaulniers updated this revision to Diff 250046. nickdesaulniers added a comment. nickdesaulniers marked an