[PATCH] D19689: Add Subjects to NoDebugAttr [NFC]

[David Blaikie via cfe-commits]

LGTM

On Thu, Apr 28, 2016 at 2:10 PM, Paul Robinson via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> probinson created this revision.
> probinson added a reviewer: aaron.ballman.
> probinson added a subscriber: cfe-commits.
>
> The 'nodebug' attribute had hand-coded constraints; replace those with a
> Subjects line in Attr.td.
> Also add a missing test to verify the attribute is okay on an Objective-C
> method.
>
> http://reviews.llvm.org/D19689
>
> Files:
>   include/clang/Basic/Attr.td
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/Sema/SemaDeclAttr.cpp
>   test/Sema/attr-nodebug.c
>   test/SemaObjC/attr-nodebug.m
>
> Index: test/SemaObjC/attr-nodebug.m
> ===================================================================
> --- test/SemaObjC/attr-nodebug.m
> +++ test/SemaObjC/attr-nodebug.m
> @@ -0,0 +1,5 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +// expected-no-diagnostics
> + at interface NSObject
> +- (void)doSomething __attribute__((nodebug));
> + at end
>

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)

Or is this not actually a new/separate codepath? In which case do we really
need the test?


> Index: test/Sema/attr-nodebug.c
> ===================================================================
> --- test/Sema/attr-nodebug.c
> +++ test/Sema/attr-nodebug.c
> @@ -3,7 +3,7 @@
>  int a __attribute__((nodebug));
>
>  void b() {
> -  int b __attribute__((nodebug)); // expected-warning {{'nodebug' only
> applies to variables with static storage duration and functions}}
> +  int b __attribute__((nodebug)); // expected-warning {{'nodebug'
> attribute only applies to functions and global variables}}
>  }
>
>  void t1() __attribute__((nodebug));
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp
> +++ lib/Sema/SemaDeclAttr.cpp
> @@ -3572,18 +3572,6 @@
>  }
>
>  static void handleNoDebugAttr(Sema &S, Decl *D, const AttributeList
> &Attr) {
> -  if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
> -    if (!VD->hasGlobalStorage())
> -      S.Diag(Attr.getLoc(),
> -             diag::warn_attribute_requires_functions_or_static_globals)
> -        << Attr.getName();
> -  } else if (!isFunctionOrMethod(D)) {
> -    S.Diag(Attr.getLoc(),
> -           diag::warn_attribute_requires_functions_or_static_globals)
> -      << Attr.getName();
> -    return;
> -  }
> -
>    D->addAttr(::new (S.Context)
>               NoDebugAttr(Attr.getRange(), S.Context,
>                           Attr.getAttributeSpellingListIndex()));
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -2504,9 +2504,6 @@
>  def warn_incomplete_encoded_type : Warning<
>    "encoding of %0 type is incomplete because %1 component has unknown
> encoding">,
>    InGroup<DiagGroup<"encode-type">>;
> -def warn_attribute_requires_functions_or_static_globals : Warning<
> -  "%0 only applies to variables with static storage duration and
> functions">,
> -  InGroup<IgnoredAttributes>;
>  def warn_gnu_inline_attribute_requires_inline : Warning<
>    "'gnu_inline' attribute requires function to be marked 'inline',"
>    " attribute ignored">,
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -973,6 +973,8 @@
>
>  def NoDebug : InheritableAttr {
>    let Spellings = [GCC<"nodebug">];
> +  let Subjects = SubjectList<[FunctionLike, ObjCMethod, GlobalVar],
> WarnDiag,
> +
> "ExpectedFunctionGlobalVarMethodOrProperty">;
>    let Documentation = [NoDebugDocs];
>  }
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160428/a462cc96/attachment-0001.html>