<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 3, 2016 at 4:38 PM, Paul Robinson via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">probinson marked 2 inline comments as done.<br>
<span class=""><br>
================<br>
Comment at: include/clang/Basic/Attr.td:86-88<br>
@@ -85,1 +85,5 @@<br>
+def NonParmVar : SubsetSubject<Var,<br>
+ [{S->getKind() != Decl::ImplicitParam &&<br>
+ S->getKind() != Decl::ParmVar &&<br>
+ S->getKind() != Decl::NonTypeTemplateParm}]>;<br>
def NonBitField : SubsetSubject<Field,<br>
----------------<br>
</span><span class="">probinson wrote:<br>
> aaron.ballman wrote:<br>
> > Can you add tests for each of these cases to ensure that the diagnostic fires on all of them?<br>
> Actually not sure how to apply an attribute to an ImplicitParam....<br>
</span>Well, this is all very exciting. I tried<br>
```<br>
template<__attribute__((nodebug)) int i> int t() { return i; }<br>
int g() { return t<2>(); }<br>
```<br>
but got no diagnostic. In fact. putting assert(false) in handleNoDebugAttr shows that it's never called. Unsurprisingly, debug info for the template parameter still appears.<br>
Maybe nobody has ever been so foolish as to try to put an attribute on a template parameter before? (It is mildly disturbing that putting an attribute in an apparently impossible place yields no diagnostic and no semantic effect.)<br>
I was obviously modeling this check on NormalVar, which (it turns out) is never used. And if you can't put attributes on template parameters or (it would seem likely) implicit parameters, then NonParmVar should check for nothing more than ParmVar.<br>
And that's what I'll do.<br>
<br>
(Marking as Done because the set of tests now matches the specified condition.)<br>
<span class=""><br>
================<br>
Comment at: test/CodeGenCXX/debug-info-nodebug.cpp:50<br>
@@ -49,1 +49,3 @@<br>
NODEBUG static int static_local = 6;<br>
+ NODEBUG const int const_local = 7;<br>
+ NODEBUG int normal_local = 8;<br>
----------------<br>
</span><span class="">dblaikie wrote:<br>
> Doesn't look like the const case is any different from the non-const case, is it?<br>
</span>Given a white-box analysis of the compiler internals, you are correct; this is in contrast to the static const/non-const handling, which *does* use different paths.<br>
I am unwilling to trust that the const/non-const paths for locals will forever use the same path. You could also look at it as "the test is the spec."<br></blockquote><div><br></div><div>But even then - what about any other property of a variable? What if it's in a nested scope instead of a top level scope? What if it's declared in a condition (if (int x = ...)). What if it's volatile? We could pick arbitrary properties that /could/ have an effect on debug info but we generally believe to be orthogonal to the feature at hand & I'd say 'const' is in the same group of things. (a const local variable still needs storage, etc, - the address can be escaped and accessed from elsewhere and determined to be unique - and both variables in this example could be optimized away entirely by the compiler because they're unused, so the const is no worse off there in that theoretical concern)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
================<br>
Comment at: test/CodeGenObjC/debug-info-nodebug.m:17<br>
@@ +16,3 @@<br>
+ // CHECK-NOT: !DILocalVariable(name: "strongSelf"<br>
+ __attribute__((nodebug)) __typeof(self) weakSelf = self;<br>
+ Block = [^{<br>
----------------<br>
</span><span class="">dblaikie wrote:<br>
> Is this case outside of the block interesting in some way? It doesn't look like it.<br>
</span>The attribute on "weakSelf" is what triggers the second modified path in CGDebugInfo and suppresses the DILocalVariable for that name.<br>
The attribute on "strongSelf" goes through the normal EmitDeclare path. So in that sense, it is not interesting.<br>
<br>
I should not have been so hesitant to work out what was going on here, sorry about that. My cluelessness about Objective-C knows no bounds.<br>
I'm changing the test to verify that the DILocalVariable does not appear, while the member info does still appear, and updating the comment to reflect this new knowledge.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
<a href="http://reviews.llvm.org/D19754" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19754</a><br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>