[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks.  Some minor changes and then LGTM.

I'm currently wondering if a better solution might be to just teach the Bypasses analysis about the C lifetime rule.  But we don't need to do that right now.



================
Comment at: lib/CodeGen/CGDecl.cpp:1031
+        // Skip emitting lifetime intrinsics when a label was seen in the
+        // current block and we are compiling for C.
+        if (!Bypasses.IsBypassed(&D) &&
----------------
It's probably appropriate to just rework this entire comment at this point.  Suggestion:

  If there's a jump into the lifetime of this variable, its lifetime gets broken
  up into several regions in IR, which requires more work to handle correctly.
  For now, just omit the intrinsics; this is a rare case, and it's better to just be
  conservatively correct.  PR28267.

  We have to do this in all language modes if there's a jump past the declaration.
  We also have to do it in C if there's a jump to an earlier point in the current
  block because non-VLA lifetimes begin as soon as the containing block is
  entered, not when its variables actually come into scope; suppressing the
  lifetime annotations completely in this case is unnecessarily pessimistic, but
  again, this is rare.


================
Comment at: lib/CodeGen/CodeGenFunction.h:216
+  /// Return true if a label was seen in the current block.
+  bool labelSeen() const {
+    if (CurLexicalScope)
----------------
It seems reasonable to give this a more descriptive name, like hasLabelBeenSeenInCurrentScope().


https://reviews.llvm.org/D27680