[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

In D76096#4540442 <https://reviews.llvm.org/D76096#4540442>, @nickdesaulniers wrote:

> Consider the following code:
>
>   long x [] = {1, 2, 3, 4, 5 + 4};
>
> Even with some of my recent changes, because the fast path parallel implementation doesn't currently handle BinaryOperator Exprs, we'll visit all of the ImplicitCastExpr and IntegerLiteral, then eventually figure out that we can't constant evaluate the final element (for the wrong reasons; i.e. VisitBinaryOperator not implemented).  That's a slow down because then we'll fall back to EvaluateAsLValue/EvaluateAsRValue which will then succeed.
>
> So we pretty much need a fair amount of parallel implementation for the fast path to succeed.  I'm happy to implement all that, if that's the direction you're advising?
>
> Or did you mean something else when you said "for structs and arrays?"

The given example doesn't fallback in the "bad" way: we don't create an APValue representing `x[]`.  If you look at the code for lowering InitListExpr, you'll note that it doesn't actually just recursively Visit() like we do in other places.  We do create an lvalue for "5+4", but that isn't expensive in the same way.

> Is there another way I can prove the absence of regression to you?

At this point, I'm basically convinced the regressions are only going to be in obscure cases we aren't going to find by benchmarking.  I'm sure someone will find them eventually, but we can deal with them as they come up, I think.

So LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76096/new/

https://reviews.llvm.org/D76096