probinson marked 2 inline comments as done.
================
Comment at: include/clang/Basic/Attr.td:86-88
@@ -85,1 +85,5 @@
+def NonParmVar : SubsetSubject<Var,
+ [{S->getKind() != Decl::ImplicitParam &&
+ S->getKind() != Decl::ParmVar &&
+ S->getKind() != Decl::NonTypeTemplateParm}]>;
def NonBitField : SubsetSubject<Field,
----------------
probinson wrote:
> aaron.ballman wrote:
> > Can you add tests for each of these cases to ensure that the diagnostic
> > fires on all of them?
> Actually not sure how to apply an attribute to an ImplicitParam....
Well, this is all very exciting. I tried
```
template<__attribute__((nodebug)) int i> int t() { return i; }
int g() { return t<2>(); }
```
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.
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.)
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.
And that's what I'll do.
(Marking as Done because the set of tests now matches the specified condition.)
================
Comment at: test/CodeGenCXX/debug-info-nodebug.cpp:50
@@ -49,1 +49,3 @@
NODEBUG static int static_local = 6;
+ NODEBUG const int const_local = 7;
+ NODEBUG int normal_local = 8;
----------------
dblaikie wrote:
> Doesn't look like the const case is any different from the non-const case, is
> it?
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.
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."
================
Comment at: test/CodeGenObjC/debug-info-nodebug.m:17
@@ +16,3 @@
+ // CHECK-NOT: !DILocalVariable(name: "strongSelf"
+ __attribute__((nodebug)) __typeof(self) weakSelf = self;
+ Block = [^{
----------------
dblaikie wrote:
> Is this case outside of the block interesting in some way? It doesn't look
> like it.
The attribute on "weakSelf" is what triggers the second modified path in
CGDebugInfo and suppresses the DILocalVariable for that name.
The attribute on "strongSelf" goes through the normal EmitDeclare path. So in
that sense, it is not interesting.
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.
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.
http://reviews.llvm.org/D19754
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
