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

2016-06-15 Thread Paul Robinson via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL272859: Allow 'nodebug' on local variables. (authored by probinson). Changed prior to commit: http://reviews.llvm.org/D19754?vs=60930&id=60940#toc Repository: rL LLVM http://reviews.llvm.org/D19754

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

2016-06-15 Thread David Blaikie via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. Sure, looks good - thanks. Will discuss the broader test issues later/separately. http://reviews.llvm.org/D19754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

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

2016-06-15 Thread Paul Robinson via cfe-commits
probinson marked an inline comment as done. Comment at: test/CodeGenCXX/debug-info-nodebug.cpp:50 @@ -49,2 +49,3 @@ NODEBUG static int static_local = 6; + NODEBUGint normal_local = 7; } const case removed. http://reviews.llvm.org/D19754 _

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

2016-06-15 Thread Paul Robinson via cfe-commits
probinson updated this revision to Diff 60930. probinson added a comment. Removed the apparently redundant test for a const local variable. Yes, I'm back to this after way longer than expected. I believe, for this patch specifically, the extra const local variable was the only identifiable pro

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

2016-05-17 Thread David Blaikie via cfe-commits
On Tue, May 17, 2016 at 2:30 PM, Robinson, Paul wrote: > What you are describing is what testing literature refers to as criteria > for equivalence classes. There is some level of judgment to that, yes. > > > > Yep yep, to be sure. I'm just generally trying to encourage the community > behavior

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

2016-05-17 Thread Robinson, Paul via cfe-commits
What you are describing is what testing literature refers to as criteria for equivalence classes. There is some level of judgment to that, yes. Yep yep, to be sure. I'm just generally trying to encourage the community behavior towards being both selective & thorough about testing. I have notice

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

2016-05-05 Thread David Blaikie via cfe-commits
On Thu, May 5, 2016 at 8:50 AM, Robinson, Paul wrote: > This would be a great conversation to have at the social, sadly I will > have to miss it this month. > Yeah, I don't often make it along to them, unfortunately. > > >> dblaikie wrote: > >>> Doesn't look like the const case is any differen

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

2016-05-05 Thread Robinson, Paul via cfe-commits
This would be a great conversation to have at the social, sadly I will have to miss it this month. >> 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 co

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

2016-05-04 Thread David Blaikie via cfe-commits
On Tue, May 3, 2016 at 4:38 PM, Paul Robinson via cfe-commits < cfe-commits@lists.llvm.org> wrote: > probinson marked 2 inline comments as done. > > > Comment at: include/clang/Basic/Attr.td:86-88 > @@ -85,1 +85,5 @@ > +def NonParmVar : SubsetSubject +

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

2016-05-04 Thread Aaron Ballman via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! http://reviews.llvm.org/D19754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

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

2016-05-03 Thread Paul Robinson via cfe-commits
probinson updated this revision to Diff 56079. probinson marked an inline comment as done. probinson added a comment. Correct the attribute condition, and refine the Objective-C test. http://reviews.llvm.org/D19754 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td lib/Cod

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

2016-05-03 Thread 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 : SubsetSubjectgetKind() != Decl::ImplicitParam && + S->getKind() != Decl::ParmVar && + S->ge

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

2016-05-02 Thread David Blaikie via cfe-commits
dblaikie added inline comments. 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; + NODEBUGint normal_local = 8; Doesn't look like the const case is any d

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

2016-05-02 Thread Paul Robinson via cfe-commits
probinson added inline comments. Comment at: include/clang/Basic/Attr.td:86-88 @@ -85,1 +85,5 @@ +def NonParmVar : SubsetSubjectgetKind() != Decl::ImplicitParam && + S->getKind() != Decl::ParmVar && + S->getKind() !=

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

2016-05-02 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:86-88 @@ -85,1 +85,5 @@ +def NonParmVar : SubsetSubjectgetKind() != Decl::ImplicitParam && + S->getKind() != Decl::ParmVar && + S->getKind()