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 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to