rjmccall added inline comments.
================
Comment at: lib/CodeGen/CodeGenFunction.h:217
+ /// statements.
+ llvm::SmallVector<bool, 4> LabelSeenStack;
+
----------------
ahatanak wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Shouldn't this be maintained by some existing scoping structure like
> > > LexicalScope?
> > I think I need a structure that gets pushed and popped when we enter and
> > exit a compound statement, but I couldn't find one in CodeGenFunction.
> >
> > Adding a flag to LexicalScope would fail to disable lifetime markers in
> > some cases. For example, in the code below (which I think is valid), the
> > lifetime markers of "i" wouldn't get disabled because a LexicalScope is
> > pushed at the beginning of CodeGenFunction::EmitForStmt:
> >
> > ```
> > void foo2(int *);
> > int c;
> >
> > void foo1(int n) {
> > int *p = 0;
> >
> > label1:
> > foo2(p);
> > for (int i = 0; i < n; ++i)
> > if (i == 5)
> > p = &i;
> >
> > if (c)
> > goto label1;
> > }
> > ```
> >
> > If we want to avoid introducing new structures to track labels, perhaps we
> > can just check whether a label was seen so far in the current function
> > rather than the current compound statement, but I'm not sure how much
> > impact that would have on performance.
> Alternatively, add a flag to LexicalScope that indicates it was created in
> EmitCompoundStmtWithoutScope. lifetime markers would be disabled if the flag
> of the enclosing CompoundStmt's LexicalScope were set.
The C language rule is not tied specifically to compound statements; it is tied
to "blocks". Selection (if/switch) and iteration (for/do/while) statements are
also blocks. So the code is not valid because the lifetime of 'i' is tied to
the block of the for statement. This is basically what we currently track with
LexicalScope.
The C++ language rule is similar but stricter.
================
Comment at: lib/CodeGen/CodeGenFunction.h:236
+ LabelSeenStack.back() = true;
+ }
+
----------------
ahatanak wrote:
> rjmccall wrote:
> > A label in a nested scope acts like a label in outer scopes, too.
> I think you are suggesting setting all the flags on the stack rather than
> just the top (or maybe not?), but I'm not sure why that's necessary. Could
> you elaborate on your comment a little more and show me a simple example when
> this code would fail to disable lifetime markers when it should?
>
> The boolean flag set to true is propagated to the outer scope's flag when we
> have completed processing the compound statement and LabelSeenRAII's
> destructor is called. So, for example, if I enclose label1 in function
> jump_backward_over_declaration of the test case,
> IRGen disables lifetime markers for "i" even though "i" is declared in the
> outer compound statement of label1.
Okay. And since it's not possible to query a non-active scope's flag, this
works.
https://reviews.llvm.org/D27680
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
