[attributes][analyzer] Generalize [[clang::suppress]] to declarations. (llvm#80371)

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.

Author: haoNoQ

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">]>];
+  // There's no fundamental reason why we can't simply accept all Decls
+  // but let's make a short list so that to avoid supporting something weird
+  // by accident. We can always expand the list later.
+  let Subjects = SubjectList<[
+    Stmt, Var, Field, ObjCProperty, Function, ObjCMethod, Record, ObjCInterface,
+    ObjCImplementation, Namespace, Empty
+  ], ErrorDiag, "variables, functions, structs, interfaces, and namespaces">;
   let Documentation = [SuppressDocs];
 }
 

clang/include/clang/Basic/AttrDocs.td

@@ -5321,6 +5321,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++
+
+  class [[clang::suppress]] C {
+    int foo() {
+      int *x = nullptr;
+      ...
+      return *x;  // warnings suppressed in the entire class scope
+    }
+
+    int bar();
+  };
+
+  int C::bar() {
+    int *x = nullptr;
+    ...
+    return *x;  // warning NOT suppressed! - not lexically nested in 'class C{}'
+  }
+
 Some static analysis warnings are accompanied by one or more notes, and the
 line of code against which the warning is emitted isn't necessarily the best
 for suppression purposes. In such cases the tools are allowed to implement

clang/lib/Sema/SemaDecl.cpp

@@ -2960,6 +2960,9 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
         S.mergeHLSLNumThreadsAttr(D, *NT, NT->getX(), NT->getY(), NT->getZ());
   else if (const auto *SA = dyn_cast<HLSLShaderAttr>(Attr))
     NewAttr = S.mergeHLSLShaderAttr(D, *SA, SA->getType());
+  else if (const auto *SupA = dyn_cast<SuppressAttr>(Attr))
+    // Do nothing. Each redeclaration should be suppressed separately.
+    NewAttr = nullptr;
   else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
     NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
 

clang/lib/Sema/SemaDeclAttr.cpp

@@ -5260,11 +5260,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;
-  } else if (!isa<VarDecl>(D)) {
-    // Analyzer suppression applies only to variables and statements.
-    S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type_str)
-        << AL << 0 << "variables and statements";
-    return;
   }
 
   std::vector<StringRef> DiagnosticIdentifiers;

clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp

@@ -161,8 +161,8 @@ static void checkObjCUnusedIvar(const ObjCImplementationDecl *D,
 
       PathDiagnosticLocation L =
           PathDiagnosticLocation::create(Ivar, BR.getSourceManager());
-      BR.EmitBasicReport(D, Checker, "Unused instance variable", "Optimization",
-                         os.str(), L);
+      BR.EmitBasicReport(ID, Checker, "Unused instance variable",
+                         "Optimization", os.str(), L);
     }
 }
 

clang/lib/StaticAnalyzer/Core/BugSuppression.cpp

@@ -82,12 +82,12 @@ class CacheInitializer : public RecursiveASTVisitor<CacheInitializer> {
     CacheInitializer(ToInit).TraverseDecl(const_cast<Decl *>(D));
   }
 
-  bool VisitVarDecl(VarDecl *VD) {
+  bool VisitDecl(Decl *D) {
     // Bug location could be somewhere in the init value of
     // a freshly declared variable.  Even though it looks like the
     // user applied attribute to a statement, it will apply to a
     // variable declaration, and this is where we check for it.
-    return VisitAttributedNode(VD);
+    return VisitAttributedNode(D);
   }
 
   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();
+  } else {
+    // This is the fast path. However, we should still consider the topmost
+    // declaration that isn't TranslationUnitDecl, because we should respect
+    // attributes on the entire declaration chain.
+    while (true) {
+      // Use the "lexical" parent. Eg., if the attribute is on a class, suppress
+      // warnings in inline methods but not in out-of-line methods.
+      const Decl *Parent =
+          dyn_cast_or_null<Decl>(DeclWithIssue->getLexicalDeclContext());
+      if (Parent == nullptr || isa<TranslationUnitDecl>(Parent))
+        break;
+
+      DeclWithIssue = Parent;
+    }
   }
 
   // While some warnings are attached to AST nodes (mostly path-sensitive

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 {
+  virtual ~SuppressedDerivedWithVirtualDtor() {}
+};
 
+// 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<class T>
 struct DerivedClassTmpl : T { };

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]
+
+  // Confirm that the checker respects [[clang::suppress]].
+  RefCountable* suppressed_ref_countable = nullptr;
+  [[clang::suppress]] auto foo5 = [suppressed_ref_countable](){};
+  // CHECK-NOT: warning: Captured raw-pointer 'suppressed_ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
 }
 
 void references() {

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]}}
+    [[clang::suppress]] auto *baz_suppressed = provide_ref_ctnbl(); // no-warning
   }
 };
 } // namespace auto_keyword

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'}}
 
+    [[clang::suppress]]
+    RefCountable* a_suppressed = nullptr;
+
   protected:
     RefPtr<RefCountable> b;
 
@@ -25,8 +28,14 @@ namespace members {
   };
 
   void forceTmplToInstantiate(FooTmpl<RefCountable>) {}
+
+  struct [[clang::suppress]] FooSuppressed {
+  private:
+    RefCountable* a = nullptr;
+  };
 }
 
+
 namespace ignore_unions {
   union Foo {
     RefCountable* a;