This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by rsmith on Jun 11 2019, 12:42 PM.

Details

Summary

When a variable is named in a context where we can't directly emit a
reference to it (because we don't know for sure that it's going to be
defined, or it's from an enclosing function and not captured, or the
reference might not "work" for some reason), we emit a copy of the
variable as a global and use that for the known-to-be-read-only access.

Diff Detail

Repository
rL LLVM

Event Timeline

rsmith created this revision.Jun 11 2019, 12:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2019, 12:42 PM
Herald added a subscriber: jdoerfert. · View Herald Transcript
rjmccall added inline comments.Jun 12 2019, 11:31 AM
lib/CodeGen/CGExpr.cpp
1429 ↗(On Diff #204131)

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.

lib/Sema/SemaExpr.cpp
15808 ↗(On Diff #204131)

This is really cute; I might steal this idea. That said, it's probably worth explaining the lifetime mechanics here so that non-experts can understand how this works.

rsmith updated this revision to Diff 204589.Jun 13 2019, 11:17 AM
rsmith marked 2 inline comments as done.

Updated comment, fixed typo, added use of [[clang::lifetimebound]].

rsmith added inline comments.Jun 13 2019, 11:21 AM
lib/CodeGen/CGExpr.cpp
1429 ↗(On Diff #204131)

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.

lib/Sema/SemaExpr.cpp
15808 ↗(On Diff #204131)

Done. I also added a [[clang::lifetimebound]] attribute so we'll get a warning if this is misused.

Minor comment request, then LGTM.

lib/CodeGen/CGDecl.cpp
1102 ↗(On Diff #204589)

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.

lib/CodeGen/CGExpr.cpp
1429 ↗(On Diff #204131)

Hmm, alright.

rsmith marked an inline comment as done.Jun 13 2019, 11:56 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 13 2019, 11:57 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2019, 11:57 AM