[PATCH] D19754: Allow 'nodebug' on local variables

[Paul Robinson via cfe-commits]

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