[attributes][analyzer] Generalize [[clang::suppress]] to declarations. by haoNoQ · Pull Request #80371 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Artem Dergachev (haoNoQ)

Changes

The attribute is now allowed on an assortment of declarations, to suppress warnings related to declarations themselves, or all warnings in the lexical scope of the declaration.

I don't necessarily see a reason to have a list at all, but it does look as if some of those more niche items aren't properly supported by the compiler itself so let's maintain a short safe list for now.

The initial implementation raised a question whether the attribute should apply to lexical declaration context vs. "actual" declaration context. I'm using "lexical" here because it results in less warnings suppressed, which is the conservative behavior: we can always expand it later if we think this is wrong, without breaking any existing code. I also think that this is the correct behavior that we will probably never want to change, given that the user typically desires to keep the suppressions as localized as possible.


Patch is 21.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80371.diff

18 Files Affected:

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 58838b01b4fd7..1b37b01ba6a3f 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2891,6 +2891,13 @@ def Suppress : DeclOrStmtAttr { let Spellings = [CXX11<"gsl", "suppress">, Clang<"suppress">]; let Args = [VariadicStringArgument<"DiagnosticIdentifiers">]; let Accessors = [Accessor<"isGSL", [CXX11<"gsl", "suppress">]>];

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index e02a1201e2ad7..a98d4b1f8d84d 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -5313,6 +5313,29 @@ Putting the attribute on a compound statement suppresses all warnings in scope: } }

+The attribute can also be placed on entire declarations of functions, classes, +variables, member variables, and so on, to suppress warnings related +to the declarations themselves. When used this way, the attribute additionally +suppresses all warnings in the lexical scope of the declaration: + +.. code-block:: c++ +

else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr)) NewAttr = cast(Attr->clone(S.Context));

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 069571fcf7864..3291ad732e98d 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5245,11 +5245,6 @@ static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // Suppression attribute with GSL spelling requires at least 1 argument. if (!AL.checkAtLeastNumArgs(S, 1)) return;

}

std::vector DiagnosticIdentifiers; diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp index 1c2d84254d464..4f8750d9f1966 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp @@ -161,7 +161,7 @@ static void checkObjCUnusedIvar(const ObjCImplementationDecl *D,

   PathDiagnosticLocation L =
       PathDiagnosticLocation::create(Ivar, BR.getSourceManager());

}

bool VisitAttributedStmt(AttributedStmt *AS) { @@ -147,6 +147,20 @@ bool BugSuppression::isSuppressed(const PathDiagnosticLocation &Location, // done as well as perform a lot of work we'll never need. // Gladly, none of our on-by-default checkers currently need it. DeclWithIssue = ACtx.getTranslationUnitDecl();

}

// While some warnings are attached to AST nodes (mostly path-sensitive diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp index 1fc59c108b0e8..5cf7e7614d06e 100644 --- a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp +++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp @@ -13,7 +13,17 @@ struct DerivedWithVirtualDtor : RefCntblBase { virtual ~DerivedWithVirtualDtor() {} };

+// Confirm that the checker respects [[clang::suppress]] +struct [[clang::suppress]] SuppressedDerived : RefCntblBase { }; +struct [[clang::suppress]] SuppressedDerivedWithVirtualDtor : RefCntblBase {

+// FIXME: Support attributes on base specifiers? Currently clang +// doesn't support such attributes at all, even though it knows +// how to parse them. +// +// struct SuppressedBaseSpecDerived : [[clang::suppress]] RefCntblBase { };

template struct DerivedClassTmpl : T { }; diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp index 30798793ceab1..27e0a74d583cd 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp @@ -15,6 +15,11 @@ void raw_ptr() { // CHECK-NEXT:{{^ | }} ^ auto foo4 = ={ (void) ref_countable; }; // CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker] +

void references() { diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 8694d5fb85b8b..0fcd3b21376ca 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -60,6 +60,7 @@ class Foo { // expected-warning@-1{{Local variable 'baz' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} auto *baz2 = this->provide_ref_ctnbl(); // expected-warning@-1{{Local variable 'baz2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}

} }; } // namespace auto_keyword diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp index a0ea61e0e2a13..108d5effdd2e8 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp @@ -8,6 +8,9 @@ namespace members { RefCountable* a = nullptr; // expected-warning@-1{{Member variable 'a' in 'members::Foo' is a raw pointer to ref-countable type 'RefCountable'}}

@@ -25,8 +28,14 @@ namespace members { };

void forceTmplToInstantiate(FooTmpl) {} +

diff --git a/clang/test/Analysis/ObjCRetSigs.m b/clang/test/Analysis/ObjCRetSigs.m index 97d33f9f5467b..f92506a834195 100644 --- a/clang/test/Analysis/ObjCRetSigs.m +++ b/clang/test/Analysis/ObjCRetSigs.m @@ -4,10 +4,12 @@

@interface MyBase -(long long)length; +-(long long)suppressedLength; @end

@interface MySub : MyBase{} -(double)length; +-(double)suppressedLength; @end

@implementation MyBase @@ -15,6 +17,10 @@ -(long long)length{ printf("Called MyBase -length;\n"); return 3; } +-(long long)suppressedLength{

@implementation MySub @@ -22,4 +28,8 @@ -(double)length{ // expected-warning{{types are incompatible}} printf("Called MySub -length;\n"); return 3.3; } +-(double)suppressedLength [[clang::suppress]]{ // no-warning

+@interface SuppressedMissingInvalidationMethod : Foo +@property (assign) [[clang::suppress]] SuppressedMissingInvalidationMethod *foobar16_warn; +// FIXME: Suppression should have worked but decl-with-issue is the ivar, not the property. +#if RUN_IVAR_INVALIDATION +// expected-warning@-3 {{Property foobar16_warn needs to be invalidated; no invalidation method is defined in the @implementation for SuppressedMissingInvalidationMethod}} +#endif + +@end +@implementation SuppressedMissingInvalidationMethod +@end + @interface MissingInvalidationMethod2 : Foo { Foo *Ivar1; #if RUN_IVAR_INVALIDATION @@ -290,8 +301,10 @@ @implementation MissingInvalidationMethodDecl2 @end

@interface InvalidatedInPartial : SomeInvalidationImplementingObject {

+} + +// Another instance of the same namespace. +namespace suppressed_namespace {

-// TODO: reassess when we decide what to do with declaration annotations -void malloc_leak_suppression_2_2() /* SUPPRESS */ { +void malloc_leak_suppression_2_2() SUPPRESS { int *x = (int *)malloc(sizeof(int)); *x = 42; -} // expected-warning{{Potential leak of memory pointed to by 'x'}} +} // no-warning

-// TODO: reassess when we decide what to do with declaration annotations -/* SUPPRESS */ void malloc_leak_suppression_2_3() { +SUPPRESS void malloc_leak_suppression_2_3() { int *x = (int *)malloc(sizeof(int)); *x = 42; -} // expected-warning{{Potential leak of memory pointed to by 'x'}} +} // no-warning

void malloc_leak_suppression_2_4(int cond) { int *x = (int *)malloc(sizeof(int)); @@ -233,20 +231,15 @@ - (void)methodWhichMayFail:(NSError **)error {

@interface TestSuppress : UIResponder { } -// TODO: reassess when we decide what to do with declaration annotations -@property(copy) /* SUPPRESS */ NSMutableString *mutableStr; -// expected-warning@-1 {{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}} +@property(copy) SUPPRESS NSMutableString *mutableStr; // no-warning @end @implementation TestSuppress

-// TODO: reassess when we decide what to do with declaration annotations -- (BOOL)resignFirstResponder /* SUPPRESS */ { +- (BOOL)resignFirstResponder SUPPRESS { // no-warning return 0; -} // expected-warning {{The 'resignFirstResponder' instance method in UIResponder subclass 'TestSuppress' is missing a [super resignFirstResponder] call}} +}

-// TODO: reassess when we decide what to do with declaration annotations -- (void)methodWhichMayFail:(NSError **)error /* SUPPRESS */ {

@@ -269,3 +262,40 @@ void ast_checker_suppress_1() { struct ABC *Abc; SUPPRESS { Abc = (struct ABC *)&Ab; } } + +SUPPRESS int suppressed_function() {

+// Confirm that the checker respects [[clang::suppress]]. +@interface TestC { +@private

+// This doesn't really suppress anything but why not? [[clang::suppress]]; -// expected-error@-1 {{'suppress' attribute only applies to variables and statements}}

namespace N { [[clang::suppress("in-a-namespace")]]; -// expected-error@-1 {{'suppress' attribute only applies to variables and statements}} } // namespace N

[[clang::suppress]] int global = 42;

[[clang::suppress]] void foo() {

@@ -56,7 +54,11 @@ namespace N { }

class [[clang::suppress("type.1")]] V {

@@ -28,23 +27,19 @@ SUPPRESS1 switch (a) { // no-warning // GNU-style ... [truncated]