Add [[clang::suppress(rule, ...)]] attribute

Summary:
This patch implements parsing of [[clang::suppress(rule, ...)]]
and [[gsl::suppress(rule, ...)]] attributes.

C++ Core Guidelines depend heavily on tool support for
rule enforcement. They also propose a way to suppress
warnings [1] which is by annotating any ancestor in AST
with the C++11 attribute [[gsl::suppress(rule1,...)]].
To have a mechanism to suppress non-C++ Core
Guidelines specific, an additional spelling of [[clang::suppress]]
is defined.

For example, to suppress the warning cppcoreguidelines-slicing,
one could do
```
[[clang::suppress("cppcoreguidelines-slicing")]]
void f() { ... code that does slicing ... }
```
or
```
void g() {
  Derived b;
  [[clang::suppress("cppcoreguidelines-slicing")]]
  Base a{b};
  [[clang::suppress("cppcoreguidelines-slicing")]] {
    doSomething();
    Base a2{b};
  }
}
```

This parsing can then be used by clang-tidy, which includes multiple
C++ Core Guidelines rules, to suppress warnings (see
https://reviews.llvm.org/D24888).
For the exact naming of the rule in the attribute, there
are different possibilities, which will be defined in the
corresponding clang-tidy patch.

Currently, clang-tidy supports suppressing of warnings through "//
NOLINT" comments. There are some advantages that the attribute has:
- Suppressing specific warnings instead of all warnings
- Suppressing warnings in a block (namespace, function, compound
  statement)
- Code formatting may split a statement into multiple lines,
  thus a "// NOLINT" comment may be on the wrong line

I'm looking forward to your comments!

[1] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#inforce-enforcement

Reviewers: alexfh, aaron.ballman, rsmith

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D24886

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@298880 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index db8e073d006941ac2bcce3006354f5d1cb822daa..cc1d4b2f9d5f5b755cc1727eba75689a95d2dded 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -1545,6 +1545,12 @@ def SwiftIndirectResult : ParameterABIAttr {
   let Documentation = [SwiftIndirectResultDocs];
 }
 
+def Suppress : StmtAttr {
+  let Spellings = [CXX11<"gsl", "suppress">];
+  let Args = [VariadicStringArgument<"DiagnosticIdentifiers">];
+  let Documentation = [SuppressDocs];
+}
+
 def SysVABI : InheritableAttr {
   let Spellings = [GCC<"sysv_abi">];
 //  let Subjects = [Function, ObjCMethod];

diff --git a/include/clang/Basic/AttrDocs.td b/include/clang/Basic/AttrDocs.td
index 23b9bb7188e7864ad0ca69a363b224dcc617c692..fce99b9ecf70dc145d8dbfd56eb801ac5f1e7432 100644 (file)
--- a/include/clang/Basic/AttrDocs.td
+++ b/include/clang/Basic/AttrDocs.td
@@ -2771,6 +2771,32 @@ optimizations like C++'s named return value optimization (NRVO).
   }];
 }
 
+def SuppressDocs : Documentation {
+  let Category = DocCatStmt;
+  let Content = [{
+The ``[[gsl::suppress]]`` attribute suppresses specific
+clang-tidy diagnostics for rules of the `C++ Core Guidelines`_ in a portable
+way. The attribute can be attached to declarations, statements, and at
+namespace scope.
+
+.. code-block:: c++
+
+  [[gsl::suppress("Rh-public")]]
+  void f_() {
+    int *p;
+    [[gsl::suppress("type")]] {
+      p = reinterpret_cast<int*>(7);
+    }
+  }
+  namespace N {
+    [[clang::suppress("type", "bounds")]];
+    ...
+  }
+
+.. _`C++ Core Guidelines`: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#inforce-enforcement
+  }];
+}
+
 def AbiTagsDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index 31210264aff3b81ccc8f1cceed91d8c5f3d4ac45..17ce7ac3489a9c82c639e739f308ea84fac1fd77 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -4097,6 +4097,26 @@ static void handleCallConvAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   }
 }
 
+static void handleSuppressAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
+    return;
+
+  std::vector<StringRef> DiagnosticIdentifiers;
+  for (unsigned I = 0, E = Attr.getNumArgs(); I != E; ++I) {
+    StringRef RuleName;
+
+    if (!S.checkStringLiteralArgumentAttr(Attr, I, RuleName, nullptr))
+      return;
+
+    // FIXME: Warn if the rule name is unknown. This is tricky because only
+    // clang-tidy knows about available rules.
+    DiagnosticIdentifiers.push_back(RuleName);
+  }
+  D->addAttr(::new (S.Context) SuppressAttr(
+      Attr.getRange(), S.Context, DiagnosticIdentifiers.data(),
+      DiagnosticIdentifiers.size(), Attr.getAttributeSpellingListIndex()));
+}
+
 bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC, 
                                 const FunctionDecl *FD) {
   if (attr.isInvalid())
@@ -6156,6 +6176,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case AttributeList::AT_PreserveAll:
     handleCallConvAttr(S, D, Attr);
     break;
+  case AttributeList::AT_Suppress:
+    handleSuppressAttr(S, D, Attr);
+    break;
   case AttributeList::AT_OpenCLKernel:
     handleSimpleAttribute<OpenCLKernelAttr>(S, D, Attr);
     break;

diff --git a/lib/Sema/SemaStmtAttr.cpp b/lib/Sema/SemaStmtAttr.cpp
index 01fa856132d7d4e5c5c94a7458125c4290acd01b..4ee3412170a85cb641cc6f3109d4a4de5cf95437 100644 (file)
--- a/lib/Sema/SemaStmtAttr.cpp
+++ b/lib/Sema/SemaStmtAttr.cpp
@@ -53,6 +53,31 @@ static Attr *handleFallThroughAttr(Sema &S, Stmt *St, const AttributeList &A,
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleSuppressAttr(Sema &S, Stmt *St, const AttributeList &A,
+                                SourceRange Range) {
+  if (A.getNumArgs() < 1) {
+    S.Diag(A.getLoc(), diag::err_attribute_too_few_arguments)
+        << A.getName() << 1;
+    return nullptr;
+  }
+
+  std::vector<StringRef> DiagnosticIdentifiers;
+  for (unsigned I = 0, E = A.getNumArgs(); I != E; ++I) {
+    StringRef RuleName;
+
+    if (!S.checkStringLiteralArgumentAttr(A, I, RuleName, nullptr))
+      return nullptr;
+
+    // FIXME: Warn if the rule name is unknown. This is tricky because only
+    // clang-tidy knows about available rules.
+    DiagnosticIdentifiers.push_back(RuleName);
+  }
+
+  return ::new (S.Context) SuppressAttr(
+      A.getRange(), S.Context, DiagnosticIdentifiers.data(),
+      DiagnosticIdentifiers.size(), A.getAttributeSpellingListIndex());
+}
+
 static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
                                 SourceRange) {
   IdentifierLoc *PragmaNameLoc = A.getArgAsIdent(0);
@@ -279,6 +304,8 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const AttributeList &A,
     return handleLoopHintAttr(S, St, A, Range);
   case AttributeList::AT_OpenCLUnrollHint:
     return handleOpenCLUnrollHint(S, St, A, Range);
+  case AttributeList::AT_Suppress:
+    return handleSuppressAttr(S, St, A, Range);
   default:
     // if we're here, then we parsed a known attribute, but didn't recognize
     // it as a statement attribute => it is declaration attribute

diff --git a/test/Misc/ast-dump-attr.cpp b/test/Misc/ast-dump-attr.cpp
index f18fbac7d7e4a144b2b4b5c1f44aa749465688e9..07f91605e71c297212d02561ce85c8664031987d 100644 (file)
--- a/test/Misc/ast-dump-attr.cpp
+++ b/test/Misc/ast-dump-attr.cpp
@@ -179,3 +179,25 @@ void TestExternalSourceSymbolAttr5()
 __attribute__((external_source_symbol(generated_declaration, defined_in="module", language="Swift")));
 // CHECK: FunctionDecl{{.*}} TestExternalSourceSymbolAttr5
 // CHECK-NEXT: ExternalSourceSymbolAttr{{.*}} "Swift" "module" GeneratedDeclaration
+
+namespace TestSuppress {
+  [[gsl::suppress("at-namespace")]];
+  // CHECK: NamespaceDecl{{.*}} TestSuppress
+  // CHECK-NEXT: EmptyDecl{{.*}}
+  // CHECK-NEXT: SuppressAttr{{.*}} at-namespace
+  [[gsl::suppress("on-decl")]]
+  void TestSuppressFunction();
+  // CHECK: FunctionDecl{{.*}} TestSuppressFunction
+  // CHECK-NEXT SuppressAttr{{.*}} on-decl
+
+  void f() {
+      int *i;
+
+      [[gsl::suppress("on-stmt")]] {
+      // CHECK: AttributedStmt
+      // CHECK-NEXT: SuppressAttr{{.*}} on-stmt
+      // CHECK-NEXT: CompoundStmt
+        i = reinterpret_cast<int*>(7);
+      }
+    }
+}

diff --git a/test/SemaCXX/suppress.cpp b/test/SemaCXX/suppress.cpp
new file mode 100644 (file)
index 0000000..d88ae0b
--- /dev/null
+++ b/test/SemaCXX/suppress.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s -verify
+
+[[gsl::suppress("globally")]];
+
+namespace N {
+  [[gsl::suppress("in-a-namespace")]];
+}
+
+[[gsl::suppress("readability-identifier-naming")]]
+void f_() {
+  int *p;
+  [[gsl::suppress("type", "bounds")]] {
+    p = reinterpret_cast<int *>(7);
+  }
+
+  [[gsl::suppress]] int x; // expected-error {{'suppress' attribute takes at least 1 argument}}
+  [[gsl::suppress()]] int y; // expected-error {{'suppress' attribute takes at least 1 argument}}
+  int [[gsl::suppress("r")]] z; // expected-error {{'suppress' attribute cannot be applied to types}}
+  [[gsl::suppress(f_)]] float f; // expected-error {{'suppress' attribute requires a string}}
+}
+
+union [[gsl::suppress("type.1")]] U {
+  int i;
+  float f;
+};