<div dir="ltr">LGTM<br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 28, 2016 at 2:10 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 created this revision.<br>
probinson added a reviewer: aaron.ballman.<br>
probinson added a subscriber: cfe-commits.<br>
<br>
The 'nodebug' attribute had hand-coded constraints; replace those with a Subjects line in Attr.td.<br>
Also add a missing test to verify the attribute is okay on an Objective-C method.<br>
<br>
<a href="http://reviews.llvm.org/D19689" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19689</a><br>
<br>
Files:<br>
  include/clang/Basic/Attr.td<br>
  include/clang/Basic/DiagnosticSemaKinds.td<br>
  lib/Sema/SemaDeclAttr.cpp<br>
  test/Sema/attr-nodebug.c<br>
  test/SemaObjC/attr-nodebug.m<br>
<br>
Index: test/SemaObjC/attr-nodebug.m<br>
===================================================================<br>
--- test/SemaObjC/attr-nodebug.m<br>
+++ test/SemaObjC/attr-nodebug.m<br>
@@ -0,0 +1,5 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
+// expected-no-diagnostics<br>
+@interface NSObject<br>
+- (void)doSomething __attribute__((nodebug));<br>
+@end<br></blockquote><div><br></div><div>Generally tests test something other than "this program doesn't crash" - should it test that we apply the attribute correctly? (either via ast dump, or checking the resulting DWARF doesn't have debug info on the relevant entity)<br><br>Or is this not actually a new/separate codepath? In which case do we really need the test?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Index: test/Sema/attr-nodebug.c<br>
===================================================================<br>
--- test/Sema/attr-nodebug.c<br>
+++ test/Sema/attr-nodebug.c<br>
@@ -3,7 +3,7 @@<br>
 int a __attribute__((nodebug));<br>
<br>
 void b() {<br>
-  int b __attribute__((nodebug)); // expected-warning {{'nodebug' only applies to variables with static storage duration and functions}}<br>
+  int b __attribute__((nodebug)); // expected-warning {{'nodebug' attribute only applies to functions and global variables}}<br>
 }<br>
<br>
 void t1() __attribute__((nodebug));<br>
Index: lib/Sema/SemaDeclAttr.cpp<br>
===================================================================<br>
--- lib/Sema/SemaDeclAttr.cpp<br>
+++ lib/Sema/SemaDeclAttr.cpp<br>
@@ -3572,18 +3572,6 @@<br>
 }<br>
<br>
 static void handleNoDebugAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
-  if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {<br>
-    if (!VD->hasGlobalStorage())<br>
-      S.Diag(Attr.getLoc(),<br>
-             diag::warn_attribute_requires_functions_or_static_globals)<br>
-        << Attr.getName();<br>
-  } else if (!isFunctionOrMethod(D)) {<br>
-    S.Diag(Attr.getLoc(),<br>
-           diag::warn_attribute_requires_functions_or_static_globals)<br>
-      << Attr.getName();<br>
-    return;<br>
-  }<br>
-<br>
   D->addAttr(::new (S.Context)<br>
              NoDebugAttr(Attr.getRange(), S.Context,<br>
                          Attr.getAttributeSpellingListIndex()));<br>
Index: include/clang/Basic/DiagnosticSemaKinds.td<br>
===================================================================<br>
--- include/clang/Basic/DiagnosticSemaKinds.td<br>
+++ include/clang/Basic/DiagnosticSemaKinds.td<br>
@@ -2504,9 +2504,6 @@<br>
 def warn_incomplete_encoded_type : Warning<<br>
   "encoding of %0 type is incomplete because %1 component has unknown encoding">,<br>
   InGroup<DiagGroup<"encode-type">>;<br>
-def warn_attribute_requires_functions_or_static_globals : Warning<<br>
-  "%0 only applies to variables with static storage duration and functions">,<br>
-  InGroup<IgnoredAttributes>;<br>
 def warn_gnu_inline_attribute_requires_inline : Warning<<br>
   "'gnu_inline' attribute requires function to be marked 'inline',"<br>
   " attribute ignored">,<br>
Index: include/clang/Basic/Attr.td<br>
===================================================================<br>
--- include/clang/Basic/Attr.td<br>
+++ include/clang/Basic/Attr.td<br>
@@ -973,6 +973,8 @@<br>
<br>
 def NoDebug : InheritableAttr {<br>
   let Spellings = [GCC<"nodebug">];<br>
+  let Subjects = SubjectList<[FunctionLike, ObjCMethod, GlobalVar], WarnDiag,<br>
+                              "ExpectedFunctionGlobalVarMethodOrProperty">;<br>
   let Documentation = [NoDebugDocs];<br>
 }<br>
<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>
<br></blockquote></div><br></div></div>