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

[Matthias Gehre via cfe-commits]

mgehre created this revision.
mgehre added reviewers: alexfh, aaron.ballman, rsmith.
mgehre added a subscriber: cfe-commits.

This patch implements parsing of [[clang::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 [[suppress(rule1,...)]].
I understand that clang has a policy to put all non-standard
attributes in clang namespace, thus here it is named
[[clang::suppress(rule1, ...)]].

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 (patch upcoming).
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:
- Supressing 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 could imagine to later extend this attribute further (if agreed)
- for suppressing clang-tidy warnings in general
- for suppressing clang warnings in general.

I'm looking forward to your comments!

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

https://reviews.llvm.org/D24886

Files:
  include/clang/Basic/Attr.td
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaStmtAttr.cpp

Index: lib/Sema/SemaStmtAttr.cpp
===================================================================
--- lib/Sema/SemaStmtAttr.cpp
+++ lib/Sema/SemaStmtAttr.cpp
@@ -53,6 +53,25 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleSuppressAttr(Sema &S, Stmt *St, const AttributeList &A,
+                                SourceRange Range) {
+  std::vector<StringRef> Rules;
+
+  for (unsigned I = 0, E = A.getNumArgs(); I != E; ++I) {
+    StringRef RuleName;
+    SourceLocation LiteralLoc;
+
+    if (!S.checkStringLiteralArgumentAttr(A, I, RuleName, &LiteralLoc))
+      return nullptr;
+
+    Rules.push_back(RuleName);
+  }
+
+  SuppressAttr Attr(A.getRange(), S.Context, Rules.data(), Rules.size(),
+                    A.getAttributeSpellingListIndex());
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
                                 SourceRange) {
   IdentifierLoc *PragmaNameLoc = A.getArgAsIdent(0);
@@ -283,6 +302,8 @@
     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
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -3838,6 +3838,24 @@
   }
 }
 
+static void handleSuppressAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+  std::vector<StringRef> Rules;
+
+  for (unsigned I = 0, E = Attr.getNumArgs(); I != E; ++I) {
+    StringRef RuleName;
+    SourceLocation LiteralLoc;
+
+    if (!S.checkStringLiteralArgumentAttr(Attr, I, RuleName, &LiteralLoc))
+      return;
+
+    Rules.push_back(RuleName);
+  }
+  D->addAttr(::new (S.Context) SuppressAttr(
+      Attr.getRange(), S.Context, Rules.data(), Rules.size(),
+      Attr.getAttributeSpellingListIndex()));
+}
+
+
 bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC, 
                                 const FunctionDecl *FD) {
   if (attr.isInvalid())
@@ -5768,6 +5786,9 @@
   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;
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1451,6 +1451,12 @@
   let Documentation = [SwiftIndirectResultDocs];
 }
 
+def Suppress : StmtAttr {
+  let Spellings = [CXX11<"clang", "suppress">];
+  let Args = [VariadicStringArgument<"Rules">];
+  let Documentation = [Undocumented];
+}
+
 def SysVABI : InheritableAttr {
   let Spellings = [GCC<"sysv_abi">];
 //  let Subjects = [Function, ObjCMethod];


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D24886.72359.patch
Type: text/x-patch
Size: 3125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160923/44b41be6/attachment-0001.bin>