r298880 - Add [[clang::suppress(rule, ...)]] attribute
Matthias Gehre via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 27 12:45:24 PDT 2017
Author: mgehre
Date: Mon Mar 27 14:45:24 2017
New Revision: 298880
URL: http://llvm.org/viewvc/llvm-project?rev=298880&view=rev
Log:
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
Added:
cfe/trunk/test/SemaCXX/suppress.cpp
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaStmtAttr.cpp
cfe/trunk/test/Misc/ast-dump-attr.cpp
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=298880&r1=298879&r2=298880&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon Mar 27 14:45:24 2017
@@ -1545,6 +1545,12 @@ def SwiftIndirectResult : ParameterABIAt
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];
Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=298880&r1=298879&r2=298880&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon Mar 27 14:45:24 2017
@@ -2771,6 +2771,32 @@ optimizations like C++'s named return va
}];
}
+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 = [{
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=298880&r1=298879&r2=298880&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Mar 27 14:45:24 2017
@@ -4097,6 +4097,26 @@ static void handleCallConvAttr(Sema &S,
}
}
+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
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;
Modified: cfe/trunk/lib/Sema/SemaStmtAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAttr.cpp?rev=298880&r1=298879&r2=298880&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmtAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAttr.cpp Mon Mar 27 14:45:24 2017
@@ -53,6 +53,31 @@ static Attr *handleFallThroughAttr(Sema
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 &
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
Modified: cfe/trunk/test/Misc/ast-dump-attr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-attr.cpp?rev=298880&r1=298879&r2=298880&view=diff
==============================================================================
--- cfe/trunk/test/Misc/ast-dump-attr.cpp (original)
+++ cfe/trunk/test/Misc/ast-dump-attr.cpp Mon Mar 27 14:45:24 2017
@@ -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);
+ }
+ }
+}
Added: cfe/trunk/test/SemaCXX/suppress.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/suppress.cpp?rev=298880&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/suppress.cpp (added)
+++ cfe/trunk/test/SemaCXX/suppress.cpp Mon Mar 27 14:45:24 2017
@@ -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;
+};
More information about the cfe-commits
mailing list