[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Minor comment request, then LGTM.



================
Comment at: lib/CodeGen/CGDecl.cpp:1102
+  // Form a simple per-variable LRU cache of these values in case we find we
+  // want to reuse them.
+  llvm::GlobalVariable *&CacheEntry = InitializerConstants[&D];
----------------
I don't think this cache is LRU in any meaningful sense; it just grows unbounded.  There's this thing about requiring the initializer to match, but that's not LRU.


================
Comment at: lib/CodeGen/CGExpr.cpp:1429
+/// for instance if a block or lambda or a member of a local class uses a
+/// const int variable or constexpr variable from an enclosing function.
 CodeGenFunction::ConstantEmission
----------------
rsmith wrote:
> rjmccall wrote:
> > Isn't the old comment correct here?  This is mandatory because of the enclosing-local-scope issues; that might be an "optimization" in the language, but it's not an optimization at the IRGen level because Sema literally is forcing us to do it. 
> In the absence of this code, we'd emit the variable as a global constant from `EmitDeclRefLValue` in the enclosing-local-scope case (as we now do in the more-complex cases), so this should never be necessary for correct code generation any more.
Hmm, alright.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63157