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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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:
> > > > You need to be more careful here; we can call ConstExprEmitter for arbitrary expressions.
> > > "Be more careful" how?
> > Here are some specific cases in which you need to be more careful because the code as-is would do the wrong thing:
> > 
> >  * when emitting a global's initializer in C++, where the value of the object denoted by the DeclRefExpr could have changed between its initialization and the expression we're currently emitting 
> >  * when emitting anything other than a global initializer in C, where the value of a global could have changed after its emission
> >  * when emitting a reference to a non-global declaration in C (local variables might change between initialization and use)
> > 
> > We would need to restrict this to the cases where the variable's value cannot possibly have changed between initialization and use.
> > 
> > In C, that's (mostly) the case for a static storage variable referenced from the initializer of a static storage variable, for a thread storage variable referenced from the initializer of a static storage variable, or for a thread storage variable referenced from the initializer of a thread storage variable. Even then, this isn't strictly correct in the presence of DSOs, but I think it should be correct if the two variables are defined in the same translation unit.
> > 
> > In C++, that's (mostly) the case when the variable is `const` or `constexpr` and has no mutable subobjects. (There's still the case where the reference happens outside the lifetime of the object -- for the most part we can handwave that away by saying it must be UB, but that's not true in general in the period of construction and period of destruction.)
> > 
> > In both cases, the optimization is (arguably) still wrong if there are any volatile subobjects.
> And this is why I don't want to duplicate the logic. :)
> 
> I'd rather not make different assumptions for C and C++; instead, I'd prefer to just use the intersection that's safe in both.  I'm concerned that we could come up with weird results for mixed C and C++ code, otherwise.  Also, it's easier to define the C++ rules because we can base them on the constexpr rules in the standard.
I agree we probably want the same outcome as D76169 provides, if either the performance is acceptable or we can find a way to avoid the performance cost for the cases we already accept. Perhaps we could get that outcome by ensuring that we try the CGExprConstant fast-path (at least in C compilations, maybe in general) before we consider the complete-but-slow evaluation strategy in ExprConstant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096