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