[analyzer] BugReporterVisitors: Refactor and documentation

[Charusso]

Summary

-

[Charusso]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

195

I have made every tag a small-string.

203

I have put every Profile into the header, so it is easier to see which members are not added to the Profile. I think the root cause of https://bugs.llvm.org/show_bug.cgi?id=42938 could be some issue with differentiating between visitors.

[NoQ]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

195

This way it's harder to be sure that the tags are unique. And non-unique tags may cause terrible pain while debugging.

203

I like this!

[Szelethus]

Thanks! Bug report generation seems far less of a mess than is used to be :)

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

44–45

I suspect \emoji wasn't intentional here, given that it needs another argument :^)

[Charusso]

• Make the tags robust and more unique.

[Charusso]

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

44–45

Well, it is "So Pro" (- Apple) and I really like that. Someone wrote "along a path", but we are definitely mentioning the 🐛 path.

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

-//===- BugReporterVisitors.h - Generate PathDiagnostics ---------*- C++ -*-===//
+//===- BugReporterVisitors.h - Bug explaining and suppression ---*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
-//  This file declares BugReporterVisitors, which are used to generate enhanced
-//  diagnostic traces.
+/// \file
+/// This file declares BugReporterVisitors, which are used to generate enhanced
+/// diagnostic traces. They are also useful for suppressing false positives.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H
 #define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H
 
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"

 
 class PathSensitiveBugReport;
 class BugReporterContext;
 class ExplodedNode;
 class MemRegion;
 class PathDiagnosticPiece;
 using PathDiagnosticPieceRef = std::shared_ptr<PathDiagnosticPiece>;
 
-/// BugReporterVisitors are used to add custom diagnostics along a path.
+/// BugReporterVisitors are used to add custom diagnostics along a \emoji bug
+/// path. They also could serve false positive suppression.
 class BugReporterVisitor : public llvm::FoldingSetNode {
 public:
   BugReporterVisitor() = default;
   BugReporterVisitor(const BugReporterVisitor &) = default;
   BugReporterVisitor(BugReporterVisitor &&) {}
   virtual ~BugReporterVisitor();
 
-  /// Return a diagnostic piece which should be associated with the
-  /// given node.
-  /// Note that this function does *not* get run on the very last node
-  /// of the report, as the PathDiagnosticPiece associated with the
-  /// last node should be unique.
-  /// Use {@code getEndPath} to customize the note associated with the report
-  /// end instead.
+  /// \returns A diagnostic piece which should be associated with the
+  /// given node \p N.
   ///
-  /// The last parameter can be used to register a new visitor with the given
+  /// \note This function does *not* get run on the very last node of the
+  /// report, as the PathDiagnosticPiece associated with the last node should
+  /// be unique. Use getEndPath() to customize the note associated with the end
+  /// of the report instead.
+  ///
+  /// \note The \p BR can be used to register a new visitor with the given
   /// BugReport while processing a node.
-  virtual PathDiagnosticPieceRef VisitNode(const ExplodedNode *Succ,
+  virtual PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                            BugReporterContext &BRC,
                                            PathSensitiveBugReport &BR) = 0;
 
-  /// Last function called on the visitor, no further calls to VisitNode
-  /// would follow.
+  /// Last function called on the visitor, no further calls to
+  /// BugReporterVisitor::VisitNode() would follow.
   virtual void finalizeVisitor(BugReporterContext &BRC,
                                const ExplodedNode *EndPathNode,
                                PathSensitiveBugReport &BR);
 
-  /// Provide custom definition for the final diagnostic piece on the
+  /// Provides a custom definition for the final diagnostic piece on the
   /// path - the piece, which is displayed before the path is expanded.
   ///
-  /// NOTE that this function can be implemented on at most one used visitor,
-  /// and otherwise it crahes at runtime.
+  /// \returns The last diagnostic piece.
+  ///
+  /// \note This function can be implemented on at most one used visitor,
+  /// and otherwise it crashes at runtime.
   virtual PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC,
                                             const ExplodedNode *N,
                                             PathSensitiveBugReport &BR);
 
-  virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0;
-
   /// Generates the default final diagnostic piece.
   static PathDiagnosticPieceRef
   getDefaultEndPath(const BugReporterContext &BRC, const ExplodedNode *N,
                     const PathSensitiveBugReport &BR);
+
+  /// The tag helps to determine which visitor created a PathDiagnosticPiece.
+  ///
+  /// \returns The unique tag.
+  ///
+  /// \note Every BugReporterVisitor needs to implement this method.
+  static const char *getTag() { return "None"; }
+
+  /// The usual Profile() method for FoldingSet.
+  ///
+  /// \sa https://llvm.org/doxygen/FoldingSet_8h_source.html
+  virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0;
 };
 
 namespace bugreporter {
 
 /// Specifies the type of tracking for an expression.
 enum class TrackingKind {
   /// Default tracking kind -- specifies that as much information should be
   /// gathered about the tracked expression value as possible.
   Thorough,
   /// Specifies that a more moderate tracking should be used for the expression
   /// value. This will essentially make sure that functions relevant to the it
   /// aren't pruned, but otherwise relies on the user reading the code or
   /// following the arrows.
   Condition
 };
 
 /// Attempts to add visitors to track expression value back to its point of
 /// origin.
 ///
-/// \param N A node "downstream" from the evaluation of the statement.
-/// \param E The expression value which we are tracking
+/// \param N The error node (CheckerContext::generateErrorNode()).
+/// \param E The expression value which we are tracking.
 /// \param R The bug report to which visitors should be attached.
 /// \param EnableNullFPSuppression Whether we should employ false positive
-///         suppression (inlined defensive checks, returned null).
+///        suppression (inlined defensive checks, returned null).
 ///
-/// \return Whether or not the function was able to add visitors for this
-///         statement. Note that returning \c true does not actually imply
-///         that any visitors were added.
+/// \returns Whether the function was able to add visitors for the expression
+/// \p E.
+///
+/// \note Returning \c true does not actually imply that any visitors were
+/// added.
+///
+/// \note At the moment this function is imprecise and may could not point
+/// out where the value originates from.
 bool trackExpressionValue(const ExplodedNode *N, const Expr *E,
                           PathSensitiveBugReport &R,
                           TrackingKind TKind = TrackingKind::Thorough,
                           bool EnableNullFPSuppression = true);
 
+/// Given that statement \p S represents a pointer that would be dereferenced,
+/// try to find a sub-expression from which the pointer came from.
+/// This is used for tracking down origins of a null or undefined value:
+/// "this is null because that is null because that is null" etc.
+/// We wipe away field and element offsets because they merely add offsets.
+/// We also wipe away all casts except lvalue-to-rvalue casts, because the
+/// latter represent an actual pointer dereference; however, we remove
+/// the final lvalue-to-rvalue cast before returning from this function
+/// because it demonstrates more clearly from where the pointer rvalue was
+/// loaded. Examples:
+/// ```
+///   x->y.z      ==>  x     // (lvalue)
+///   foo()->y.z  ==>  foo() // (rvalue)
+/// ```
 const Expr *getDerefExpr(const Stmt *S);
 
 } // namespace bugreporter
 
-/// Finds last store into the given region,
-/// which is different from a given symbolic value.
+/// Finds last store into the given region, which is different from the given
+/// symbolic value.
 class FindLastStoreBRVisitor final : public BugReporterVisitor {
   const MemRegion *R;
   SVal V;
   bool Satisfied = false;
 
-  /// If the visitor is tracking the value directly responsible for the
-  /// bug, we are going to employ false positive suppression.
+  // If the visitor is tracking the value directly responsible for the
+  // bug, we are going to employ false positive suppression.
   bool EnableNullFPSuppression;
 
   using TrackingKind = bugreporter::TrackingKind;
   TrackingKind TKind;
   const StackFrameContext *OriginSFC;
 
 public:
-  /// \param V We're searching for the store where \c R received this value.
-  /// \param R The region we're tracking.
+  /// \param V We are searching for the store where \c R received this value.
+  /// \param InEnableNullFPSuppression Whether to suppress false positives.
+  /// \param R The region we are tracking.
   /// \param TKind May limit the amount of notes added to the bug report.
   /// \param OriginSFC Only adds notes when the last store happened in a
   ///        different stackframe to this one. Disregarded if the tracking kind
   ///        is thorough.
   ///        This is useful, because for non-tracked regions, notes about
   ///        changes to its value in a nested stackframe could be pruned, and
   ///        this visitor can prevent that without polluting the bugpath too
   ///        much.
   FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R,
                          bool InEnableNullFPSuppression, TrackingKind TKind,
                          const StackFrameContext *OriginSFC = nullptr)
       : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression),
         TKind(TKind), OriginSFC(OriginSFC) {
     assert(R);
   }
 
-  void Profile(llvm::FoldingSetNodeID &ID) const override;
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
+
+  static const char *getTag() { return "FindLastStoreBRVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+    ID.AddPointer(R);
+    ID.Add(V);
+    ID.AddInteger(static_cast<int>(TKind));
+    ID.AddBoolean(EnableNullFPSuppression);
+  }
 };
 
 class TrackConstraintBRVisitor final : public BugReporterVisitor {
   DefinedSVal Constraint;
   bool Assumption;
   bool IsSatisfied = false;
   bool IsZeroCheck;
 
-  /// We should start tracking from the last node along the path in which the
-  /// value is constrained.
+  // We should start tracking from the last node along the path in which the
+  // value is constrained.
   bool IsTrackingTurnedOn = false;
 
 public:
   TrackConstraintBRVisitor(DefinedSVal constraint, bool assumption)
       : Constraint(constraint), Assumption(assumption),
         IsZeroCheck(!Assumption && Constraint.getAs<Loc>()) {}
 
-  void Profile(llvm::FoldingSetNodeID &ID) const override;
-
-  /// Return the tag associated with this visitor.  This tag will be used
-  /// to make all PathDiagnosticPieces created by this visitor.
-  static const char *getTag();
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
 
+  static const char *getTag() { return "TrackConstraintBRVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+    ID.Add(Constraint);
+    ID.AddBoolean(Assumption);
+    ID.AddBoolean(IsSatisfied);
+    ID.AddBoolean(IsZeroCheck);
+    ID.AddBoolean(IsTrackingTurnedOn);
+  }
+
 private:
-  /// Checks if the constraint is valid in the current state.
+  // Checks if the constraint is valid in the current state.
   bool isUnderconstrained(const ExplodedNode *N) const;
 };
 
-/// \class NilReceiverBRVisitor
 /// Prints path notes when a message is sent to a nil receiver.
 class NilReceiverBRVisitor final : public BugReporterVisitor {
 public:
   void Profile(llvm::FoldingSetNodeID &ID) const override {
-    static int x = 0;
-    ID.AddPointer(&x);
+    ID.AddPointer(getTag());
   }
 
+  static const char *getTag() { return "NilReceiverBRVisitor"; }
+
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
 
-  /// If the statement is a message send expression with nil receiver, returns
-  /// the receiver expression. Returns NULL otherwise.
+  /// \returns The receiver expression if the statement is a message send
+  /// expression with nil receiver or nullptr otherwise.
   static const Expr *getNilReceiver(const Stmt *S, const ExplodedNode *N);
 };
 
 /// Visitor that tries to report interesting diagnostics from conditions.
 class ConditionBRVisitor final : public BugReporterVisitor {
-  // FIXME: constexpr initialization isn't supported by MSVC2013.
-  constexpr static llvm::StringLiteral GenericTrueMessage =
+  static constexpr llvm::StringLiteral GenericTrueMessage =
       "Assuming the condition is true";
-  constexpr static llvm::StringLiteral GenericFalseMessage =
+  static constexpr llvm::StringLiteral GenericFalseMessage =
       "Assuming the condition is false";
 
 public:
-  void Profile(llvm::FoldingSetNodeID &ID) const override {
-    static int x = 0;
-    ID.AddPointer(&x);
-  }
-
-  /// Return the tag associated with this visitor.  This tag will be used
-  /// to make all PathDiagnosticPieces created by this visitor.
-  static const char *getTag();
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
 
   PathDiagnosticPieceRef VisitNodeImpl(const ExplodedNode *N,
                                        BugReporterContext &BRC,
                                        PathSensitiveBugReport &BR);
 
bool patternMatch(const Expr *Ex,
                     raw_ostream &Out,
                     BugReporterContext &BRC,
                     PathSensitiveBugReport &R,
                     const ExplodedNode *N,
                     Optional<bool> &prunable,
                     bool IsSameFieldName);
 
   static bool isPieceMessageGeneric(const PathDiagnosticPiece *Piece);
+
+  static const char *getTag() { return "ConditionBRVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+  }
 };
 
 /// Suppress reports that might lead to known false positives.
 ///
 /// Currently this suppresses reports based on locations of bugs.
 class LikelyFalsePositiveSuppressionBRVisitor final
     : public BugReporterVisitor {
 public:
-  static void *getTag() {
-    static int Tag = 0;
-    return static_cast<void *>(&Tag);
+  static const char *getTag() {
+    return "LikelyFalsePositiveSuppressionBRVisitor";
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     ID.AddPointer(getTag());
   }
 
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *, BugReporterContext &,
                                    PathSensitiveBugReport &) override {

 /// if the region's contents are not modified/accessed by the call.
 class UndefOrNullArgVisitor final : public BugReporterVisitor {
   /// The interesting memory region this visitor is tracking.
   const MemRegion *R;
 
 public:
   UndefOrNullArgVisitor(const MemRegion *InR) : R(InR) {}
 
+  static const char *getTag() { return "UndefOrNullArgVisitor"; }
+
   void Profile(llvm::FoldingSetNodeID &ID) const override {
-    static int Tag = 0;
-    ID.AddPointer(&Tag);
+    ID.AddPointer(getTag());
     ID.AddPointer(R);
   }
 
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
 };
 
 class SuppressInlineDefensiveChecksVisitor final : public BugReporterVisitor {
-  /// The symbolic value for which we are tracking constraints.
-  /// This value is constrained to null in the end of path.
+  // The symbolic value for which we are tracking constraints.
+  // This value is constrained to null in the end of path.
   DefinedSVal V;
 
-  /// Track if we found the node where the constraint was first added.
+  // Track if we found the node where the constraint was first added.
   bool IsSatisfied = false;
 
-  /// Since the visitors can be registered on nodes previous to the last
-  /// node in the BugReport, but the path traversal always starts with the last
-  /// node, the visitor invariant (that we start with a node in which V is null)
-  /// might not hold when node visitation starts. We are going to start tracking
-  /// from the last node in which the value is null.
+  // Since the visitors can be registered on nodes previous to the last
+  // node in the BugReport, but the path traversal always starts with the last
+  // node, the visitor invariant (that we start with a node in which V is null)
+  // might not hold when node visitation starts. We are going to start tracking
+  // from the last node in which the value is null.
   bool IsTrackingTurnedOn = false;
 
 public:
   SuppressInlineDefensiveChecksVisitor(DefinedSVal Val, const ExplodedNode *N);
 
-  void Profile(llvm::FoldingSetNodeID &ID) const override;
-
-  /// Return the tag associated with this visitor.  This tag will be used
-  /// to make all PathDiagnosticPieces created by this visitor.
-  static const char *getTag();
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *Succ,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
+
+  static const char *getTag() { return "SuppressInlineDefensiveChecksVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+    ID.Add(V);
+  }
 };
 
 /// The bug visitor will walk all the nodes in a path and collect all the
 /// constraints. When it reaches the root node, will create a refutation
 /// manager and check if the constraints are satisfiable
 class FalsePositiveRefutationBRVisitor final : public BugReporterVisitor {
-private:
   /// Holds the constraints in a given path
   ConstraintRangeTy Constraints;
 
 public:
   FalsePositiveRefutationBRVisitor();
 
-  void Profile(llvm::FoldingSetNodeID &ID) const override;
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
 
   void finalizeVisitor(BugReporterContext &BRC, const ExplodedNode *EndPathNode,
                        PathSensitiveBugReport &BR) override;
+
+  static const char *getTag() { return "FalsePositiveRefutationBRVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+    ID.Add(Constraints);
+  }
 };
 
 
 /// The visitor detects NoteTags and displays the event notes they contain.
 class TagVisitor : public BugReporterVisitor {
 public:
-  void Profile(llvm::FoldingSetNodeID &ID) const override;
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &R) override;
+
+  static const char *getTag() { return "TagVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+  }
 };
 
 } // namespace ento
 
 } // namespace clang
 
 #endif // LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

-//===- BugReporterVisitors.cpp - Helpers for reporting bugs ---------------===//
+//===- BugReporterVisitors.cpp - Bug explaining and suppression -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
-//  This file defines a set of BugReporter "visitors" which can be used to
-//  enhance the diagnostics reported for a bug.
-//
-//===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
if (B->getLHS()->getType()->isPointerType()) {
       return B->getLHS();
     } else if (B->getRHS()->getType()->isPointerType()) {
       return B->getRHS();
     }
   }
   return nullptr;
 }
 
-/// Given that expression S represents a pointer that would be dereferenced,
-/// try to find a sub-expression from which the pointer came from.
-/// This is used for tracking down origins of a null or undefined value:
-/// "this is null because that is null because that is null" etc.
-/// We wipe away field and element offsets because they merely add offsets.
-/// We also wipe away all casts except lvalue-to-rvalue casts, because the
-/// latter represent an actual pointer dereference; however, we remove
-/// the final lvalue-to-rvalue cast before returning from this function
-/// because it demonstrates more clearly from where the pointer rvalue was
-/// loaded. Examples:
-///   x->y.z      ==>  x (lvalue)
-///   foo()->y.z  ==>  foo() (rvalue)
 const Expr *bugreporter::getDerefExpr(const Stmt *S) {
   const auto *E = dyn_cast<Expr>(S);
   if (!E)
     return nullptr;
 
   while (true) {
     if (const auto *CE = dyn_cast<CastExpr>(E)) {
       if (CE->getCastKind() == CK_LValueToRValue) {
class NoStoreFuncVisitor final : public BugReporterVisitor {
   using RegionVector = SmallVector<const MemRegion *, 5>;
 
 public:
   NoStoreFuncVisitor(const SubRegion *R, bugreporter::TrackingKind TKind)
       : RegionOfInterest(R), MmrMgr(R->getMemRegionManager()),
         SM(MmrMgr.getContext().getSourceManager()),
         PP(MmrMgr.getContext().getPrintingPolicy()), TKind(TKind) {}
 
-  void Profile(llvm::FoldingSetNodeID &ID) const override {
-    static int Tag = 0;
-    ID.AddPointer(&Tag);
-    ID.AddPointer(RegionOfInterest);
-  }
-
-  void *getTag() const {
-    static int Tag = 0;
-    return static_cast<void *>(&Tag);
-  }
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BR,
                                    PathSensitiveBugReport &R) override;
 
+  static const char *getTag() { return "NoStoreFuncVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+    ID.AddPointer(RegionOfInterest);
+  }
+
 private:
   /// Attempts to find the region of interest in a given record decl,
   /// by either following the base classes or fields.
   /// Dereferences fields up to a given recursion limit.
   /// Note that \p Vec is passed by value, leading to quadratic copying cost,
   /// but it's OK in practice since its length is limited to DEREFERENCE_LIMIT.
   /// \return A chain fields leading to the region of interest or None.
   const Optional<RegionVector>
static void addMacroVisitorIfNecessary(
         const SVal V) {
     AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
     if (EnableNullFPSuppression &&
         Options.ShouldSuppressNullReturnPaths && V.getAs<Loc>())
       BR.addVisitor(std::make_unique<MacroNullReturnSuppressionVisitor>(
               R->getAs<SubRegion>(), V));
   }
 
-  void* getTag() const {
-    static int Tag = 0;
-    return static_cast<void *>(&Tag);
-  }
+  static const char *getTag() { return "MacroNullReturnSuppressionVisitor"; }
 
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     ID.AddPointer(getTag());
   }
 
 private:
   /// \return Source location of right hand side of an assignment
   /// into \c RegionOfInterest, empty optional if none found.
class ReturnVisitor : public BugReporterVisitor {
   bugreporter::TrackingKind TKind;
 
 public:
   ReturnVisitor(const StackFrameContext *Frame, bool Suppressed,
                 AnalyzerOptions &Options, bugreporter::TrackingKind TKind)
       : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
         Options(Options), TKind(TKind) {}
 
-  static void *getTag() {
-    static int Tag = 0;
-    return static_cast<void *>(&Tag);
-  }
+  static const char *getTag() { return "ReturnVisitor"; }
 
   void Profile(llvm::FoldingSetNodeID &ID) const override {
-    ID.AddPointer(ReturnVisitor::getTag());
+    ID.AddPointer(getTag());
     ID.AddPointer(CalleeSFC);
     ID.AddBoolean(EnableNullFPSuppression);
   }
 
   /// Adds a ReturnVisitor if the given statement represents a call that was
   /// inlined.
   ///
   /// This will search back through the ExplodedGraph, starting from the given
PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
     }
 
     llvm_unreachable("Invalid visit mode!");
   }
 
   void finalizeVisitor(BugReporterContext &, const ExplodedNode *,
                        PathSensitiveBugReport &BR) override {
     if (EnableNullFPSuppression && ShouldInvalidate)
-      BR.markInvalid(ReturnVisitor::getTag(), CalleeSFC);
+      BR.markInvalid(getTag(), CalleeSFC);
   }
 };
 
 } // end of anonymous namespace
 
 //===----------------------------------------------------------------------===//
 // Implementation of FindLastStoreBRVisitor.
 //===----------------------------------------------------------------------===//
 
-void FindLastStoreBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
-  static int tag = 0;
-  ID.AddPointer(&tag);
-  ID.AddPointer(R);
-  ID.Add(V);
-  ID.AddInteger(static_cast<int>(TKind));
-  ID.AddBoolean(EnableNullFPSuppression);
-}
-
 /// Returns true if \p N represents the DeclStmt declaring and initializing
 /// \p VR.
 static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
   Optional<PostStmt> P = N->getLocationAs<PostStmt>();
   if (!P)
     return false;
 
   const DeclStmt *DS = P->getStmtAs<DeclStmt>();
if (!FrameSpace) {
     return true;
   }
 
   assert(VR->getDecl()->hasLocalStorage());
   const LocationContext *LCtx = N->getLocationContext();
   return FrameSpace->getStackFrame() == LCtx->getStackFrame();
 }
 
-/// Show diagnostics for initializing or declaring a region \p R with a bad value.
+/// Show diagnostics for initializing or declaring a region \p R with a bad
+/// value.
 static void showBRDiagnostics(const char *action, llvm::raw_svector_ostream &os,
                               const MemRegion *R, SVal V, const DeclStmt *DS) {
   if (R->canPrintPretty()) {
     R->printPretty(os);
     os << " ";
   }
 
   if (V.getAs<loc::ConcreteInt>()) {
FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
 
   return std::make_shared<PathDiagnosticEventPiece>(L, os.str());
 }
 
 //===----------------------------------------------------------------------===//
 // Implementation of TrackConstraintBRVisitor.
 //===----------------------------------------------------------------------===//
 
-void TrackConstraintBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
-  static int tag = 0;
-  ID.AddPointer(&tag);
-  ID.AddBoolean(Assumption);
-  ID.Add(Constraint);
-}
-
-/// Return the tag associated with this visitor.  This tag will be used
-/// to make all PathDiagnosticPieces created by this visitor.
-const char *TrackConstraintBRVisitor::getTag() {
-  return "TrackConstraintBRVisitor";
-}
-
 bool TrackConstraintBRVisitor::isUnderconstrained(const ExplodedNode *N) const {
   if (IsZeroCheck)
     return N->getState()->isNull(Constraint).isUnderconstrained();
   return (bool)N->getState()->assume(Constraint, !Assumption);
 }
 
 PathDiagnosticPieceRef TrackConstraintBRVisitor::VisitNode(
     const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &) {

 SuppressInlineDefensiveChecksVisitor(DefinedSVal Value, const ExplodedNode *N)
     : V(Value) {
   // Check if the visitor is disabled.
   AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
   if (!Options.ShouldSuppressInlinedDefensiveChecks)
     IsSatisfied = true;
 }
 
-void SuppressInlineDefensiveChecksVisitor::Profile(
-    llvm::FoldingSetNodeID &ID) const {
-  static int id = 0;
-  ID.AddPointer(&id);
-  ID.Add(V);
-}
-
-const char *SuppressInlineDefensiveChecksVisitor::getTag() {
-  return "IDCVisitor";
-}
-
 PathDiagnosticPieceRef
 SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ,
                                                 BugReporterContext &BRC,
                                                 PathSensitiveBugReport &BR) {
   const ExplodedNode *Pred = Succ->getFirstPred();
   if (IsSatisfied)
     return nullptr;
 
class TrackControlDependencyCondBRVisitor final : public BugReporterVisitor {
   const ExplodedNode *Origin;
   ControlDependencyCalculator ControlDeps;
   llvm::SmallSet<const CFGBlock *, 32> VisitedBlocks;
 
 public:
   TrackControlDependencyCondBRVisitor(const ExplodedNode *O)
   : Origin(O), ControlDeps(&O->getCFG()) {}
 
-  void Profile(llvm::FoldingSetNodeID &ID) const override {
-    static int x = 0;
-    ID.AddPointer(&x);
-  }
-
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
                                    PathSensitiveBugReport &BR) override;
+
+  static const char *getTag() { return "TrackControlDependencyCondBRVisitor"; }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+  }
 };
 } // end of anonymous namespace
 
 static std::shared_ptr<PathDiagnosticEventPiece>
 constructDebugPieceForTrackedCondition(const Expr *Cond,
                                        const ExplodedNode *N,
                                        BugReporterContext &BRC) {
 
PathDiagnosticLocation L(Receiver, BRC.getSourceManager(),
                                      N->getLocationContext());
   return std::make_shared<PathDiagnosticEventPiece>(L, OS.str());
 }
 
 //===----------------------------------------------------------------------===//
 // Visitor that tries to report interesting diagnostics from conditions.
 //===----------------------------------------------------------------------===//
 
-/// Return the tag associated with this visitor.  This tag will be used
-/// to make all PathDiagnosticPieces created by this visitor.
-const char *ConditionBRVisitor::getTag() { return "ConditionBRVisitor"; }
-
 PathDiagnosticPieceRef
 ConditionBRVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
                               PathSensitiveBugReport &BR) {
   auto piece = VisitNodeImpl(N, BRC, BR);
   if (piece) {
     piece->setTag(getTag());
     if (auto *ev = dyn_cast<PathDiagnosticEventPiece>(piece.get()))
       ev->setPrunable(true, /* override */ false);
for (auto const &C : NewCs) {
     if (!Constraints.contains(Sym)) {
       Constraints = CF.add(Constraints, Sym, C.second);
     }
   }
 
   return nullptr;
 }
 
-void FalsePositiveRefutationBRVisitor::Profile(
-    llvm::FoldingSetNodeID &ID) const {
-  static int Tag = 0;
-  ID.AddPointer(&Tag);
-}
-
 //===----------------------------------------------------------------------===//
 // Implementation of TagVisitor.
 //===----------------------------------------------------------------------===//
 
 int NoteTag::Kind = 0;
 
-void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
-  static int Tag = 0;
-  ID.AddPointer(&Tag);
-}
-
 PathDiagnosticPieceRef TagVisitor::VisitNode(const ExplodedNode *N,
                                              BugReporterContext &BRC,
                                              PathSensitiveBugReport &R) {
   ProgramPoint PP = N->getLocation();
   const NoteTag *T = dyn_cast_or_null<NoteTag>(PP.getTag());
   if (!T)
     return nullptr;
 

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

 //===--- SarifDiagnostics.cpp - Sarif Diagnostics for Paths -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the SarifDiagnostics object.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 
 using namespace llvm;
static Importance calculateImportance(const PathDiagnosticPiece &Piece) {
   switch (Piece.getKind()) {
   case PathDiagnosticPiece::Call:
   case PathDiagnosticPiece::Macro:
   case PathDiagnosticPiece::Note:
   case PathDiagnosticPiece::PopUp:
     // FIXME: What should be reported here?
     break;
   case PathDiagnosticPiece::Event:
-    return Piece.getTagStr() == "ConditionBRVisitor" ? Importance::Important
-                                                     : Importance::Essential;
+    return Piece.getTag() == ConditionBRVisitor::getTag()
+               ? Importance::Important
   case PathDiagnosticPiece::ControlFlow:
     return Importance::Unimportant;
   }
   return Importance::Unimportant;
 }
 
 static json::Object createThreadFlow(const LangOptions &LO,
                                      const PathPieces &Pieces,