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; +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits