[analyzer] NFC: Move IssueHash to libAnalysis.

[NoQ]

Summary

IssueHash is an attempt to introduce stable warning identifiers that won't change when code around them gets moved around. Path diagnostic consumers print issue hashes for the emitted diagnostics.

This move will allow us to ultimately move path diagnostic consumers to libAnalysis.

[gribozavr]

The "bugtype" in IssueHash is specific to Static Analyzer (or at least not documented sufficiently abstractly).

[NoQ]

In D67421#1666058, @gribozavr wrote:

The "bugtype" in IssueHash is specific to Static Analyzer (or at least not documented sufficiently abstractly).

Yeah, i'll need to document this.

That's basically the warning message; it doesn't have to have anything to do with the Analyzer's BugType structure. But if the warning message contains fragile stuff (eg., mentions a name of a variable or (why??) a line number), the user may want to wipe it out of the hash, and in this case Analyzer BugTypes turn out to be fairly handy.

[NoQ]

Rename BugType to WarningMessage, add comments.

Get rid of an unnecessary SourceManager parameter.

[gribozavr]

clang/include/clang/Analysis/IssueHash.h

18

Returns an opaque identifier for a diagnostic.

This opaque identifier is intended to be stable even when the source code is changed. It allows to track diagnostics in the long term, for example, find which diagnostics are "new", maintain a database of suppressed diagnostics etc.

The identifier includes the following information:

• the normalized source text...
• ...

The identifier does not include the following information:

• the name of the file,
• the line and column number in the file,
• ...

35

I don't understand what this paragraph is talking about at all. What does it mean for a new hash to be introduced? As in, when this hashing algorithm changes?

41

Why isn't IssueLoc const?

47

The doc comment suggests that this function returns a hash as a string, but that's not what it does. It returns some sort of identifier that is then hashed.

[Szelethus]

clang/include/clang/Analysis/IssueHash.h

35

It might refer to the extreme headaches issue hash changes can cause. In D77866,a block of comments in MallocChecker.cpp talks about this, and some other discussion (D77866#2068394) in the patch as well.

[NoQ]

Rebase!

Re-do the comments. Rename GetIssueHash() to getIssueHashV1 because there's a high chance we'll want more different kinds of hashes (also starts with a small letter, while we're at it).

clang/include/clang/Analysis/IssueHash.h

35

This paragraph was saying that if a different hashing algorithm is introduced (eg., GetIssueHashV2()), the old one should still be maintained for backwards compatibility.

[vsavchenko]

Other then the naming issue, I don't see any problems with this change!

clang/lib/Analysis/IssueHash.cpp

187

I'm not a big fan of things like this in names. First of all, numbers in names look bad. Second, it is not descriptive at all! I know, I know, it is hard to come up with a name that can capture how this hash or string algorithm is different from the other ones, when there are no other ones yet. And this actually brings up the third concern, why do even need to have this suffix now, when we don't have other options?

Let's maybe postpone it till we have the actual motivation to have another method, and give them good distinctive names then. What do you think?

[NoQ]

Get rid of the "V1" suffix. Indeed, we'll just rename the API.

[vsavchenko]

Awesome, thanks!

clang/include/clang/Analysis/IssueHash.h

+//===---------- IssueHash.h - Generate identification hashes ----*- 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
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
+#define LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
+
+#include "llvm/ADT/SmallString.h"
+
+namespace clang {
+class Decl;
+class FullSourceLoc;
+class LangOptions;
+
+/// Returns an opaque identifier for a diagnostic.
+///
+/// This opaque identifier is intended to be stable even when the source code
+/// is changed. It allows to track diagnostics in the long term, for example,
+/// find which diagnostics are "new", maintain a database of suppressed
+/// diagnostics etc.
+///
+/// We may introduce more variants of issue hashes in the future
+/// but older variants will still be available for compatibility.
+///
+/// This hash is based on the following information:
+///   - Name of the checker that emitted the diagnostic.
+///   - Warning message.
+///   - Name of the enclosing declaration.
+///   - Contents of the line of code with the issue, excluding whitespace.
+///   - Column number (but not the line number! - which makes it stable).
+llvm::SmallString<32> getIssueHash(const FullSourceLoc &IssueLoc,
+                                   llvm::StringRef CheckerName,
+                                   llvm::StringRef WarningMessage,
+                                   const Decl *IssueDecl,
+                                   const LangOptions &LangOpts);
+
+/// Get the unhashed string representation of the V1 issue hash.
+/// When hashed, it becomes the actual issue hash. Useful for testing.
+/// See GetIssueHashV1() for more information.
+std::string getIssueString(const FullSourceLoc &IssueLoc,
+                           llvm::StringRef CheckerName,
+                           llvm::StringRef WarningMessage,
+                           const Decl *IssueDecl, const LangOptions &LangOpts);
+} // namespace clang
+
+#endif

clang/include/clang/StaticAnalyzer/Core/IssueHash.h

-//===---------- IssueHash.h - Generate identification hashes ----*- 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
-//
-//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
-#define LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
-
-#include "llvm/ADT/SmallString.h"
-
-namespace clang {
-class Decl;
-class SourceManager;
-class FullSourceLoc;
-class LangOptions;
-
-/// Get an MD5 hash to help identify bugs.
-///
-/// This function returns a hash that helps identify bugs within a source file.
-/// This identification can be utilized to diff diagnostic results on different
-/// snapshots of a projects, or maintain a database of suppressed diagnotics.
-///
-/// The hash contains the normalized text of the location associated with the
-/// diagnostic. Normalization means removing the whitespaces. The associated
-/// location is the either the last location of a diagnostic path or a uniqueing
-/// location. The bugtype and the name of the checker is also part of the hash.
-/// The last component is the string representation of the enclosing declaration
-/// of the associated location.
-///
-/// In case a new hash is introduced, the old one should still be maintained for
-/// a while. One should not introduce a new hash for every change, it is
-/// possible to introduce experimental hashes that may change in the future.
-/// Such hashes should be marked as experimental using a comment in the plist
-/// files.
-llvm::SmallString<32> GetIssueHash(const SourceManager &SM,
-                                   FullSourceLoc &IssueLoc,
-                                   llvm::StringRef CheckerName,
-                                   llvm::StringRef BugType, const Decl *D,
-                                   const LangOptions &LangOpts);
-
-/// Get the string representation of issue hash. See GetIssueHash() for
-/// more information.
-std::string GetIssueString(const SourceManager &SM, FullSourceLoc &IssueLoc,
-                           llvm::StringRef CheckerName, llvm::StringRef BugType,
-                           const Decl *D, const LangOptions &LangOpts);
-} // namespace clang
-
-#endif

clang/lib/Analysis/CMakeLists.txt

add_clang_library(clangAnalysis
   CallGraph.cpp
   CloneDetection.cpp
   CocoaConventions.cpp
   ConstructionContext.cpp
   Consumed.cpp
   CodeInjector.cpp
   Dominators.cpp
   ExprMutationAnalyzer.cpp
+  IssueHash.cpp
   LiveVariables.cpp
   ObjCNoReturn.cpp
   PathDiagnostic.cpp
   PostOrderCFGView.cpp
   ProgramPoint.cpp
   ReachableCode.cpp
   RetainSummaryManager.cpp
   ThreadSafety.cpp

clang/lib/Analysis/IssueHash.cpp

 //===---------- IssueHash.cpp - Generate identification hashes --*- 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
 //
 //===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
+
+#include "clang/Analysis/IssueHash.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
static StringRef GetNthLineOfFile(const llvm::MemoryBuffer *Buffer, int Line) {
 
   llvm::line_iterator LI(*Buffer, false);
   for (; !LI.is_at_eof() && LI.line_number() != Line; ++LI)
     ;
 
   return *LI;
 }
 
-static std::string NormalizeLine(const SourceManager &SM, FullSourceLoc &L,
+static std::string NormalizeLine(const SourceManager &SM, const FullSourceLoc &L,
                                  const LangOptions &LangOpts) {
   static StringRef Whitespaces = " \t\n";
 
   StringRef Str = GetNthLineOfFile(SM.getBuffer(L.getFileID(), L),
                                    L.getExpansionLineNumber());
   StringRef::size_type col = Str.find_first_not_of(Whitespaces);
   if (col == StringRef::npos)
     col = 1; // The line only contains whitespace.
if (Token.isAtStartOfLine() && NextStart++ > 0)
       continue;
     LineBuff << std::string(SM.getCharacterData(Token.getLocation()),
                             Token.getLength());
   }
 
   return LineBuff.str();
 }
 
-static llvm::SmallString<32> GetHashOfContent(StringRef Content) {
+static llvm::SmallString<32> GetMD5HashOfContent(StringRef Content) {
   llvm::MD5 Hash;
   llvm::MD5::MD5Result MD5Res;
   SmallString<32> Res;
 
   Hash.update(Content);
   Hash.final(MD5Res);
   llvm::MD5::stringifyResult(MD5Res, Res);
 
   return Res;
 }
 
-std::string clang::GetIssueString(const SourceManager &SM,
-                                  FullSourceLoc &IssueLoc,
-                                  StringRef CheckerName, StringRef BugType,
-                                  const Decl *D,
+std::string clang::getIssueString(const FullSourceLoc &IssueLoc,
+                                  StringRef CheckerName,
+                                  StringRef WarningMessage,
+                                  const Decl *IssueDecl,
                                   const LangOptions &LangOpts) {
   static StringRef Delimiter = "$";
 
   return (llvm::Twine(CheckerName) + Delimiter +
-          GetEnclosingDeclContextSignature(D) + Delimiter +
+          GetEnclosingDeclContextSignature(IssueDecl) + Delimiter +
           Twine(IssueLoc.getExpansionColumnNumber()) + Delimiter +
-          NormalizeLine(SM, IssueLoc, LangOpts) + Delimiter + BugType)
+          NormalizeLine(IssueLoc.getManager(), IssueLoc, LangOpts) +
+          Delimiter + WarningMessage)
       .str();
 }
 
-SmallString<32> clang::GetIssueHash(const SourceManager &SM,
-                                    FullSourceLoc &IssueLoc,
-                                    StringRef CheckerName, StringRef BugType,
-                                    const Decl *D,
+SmallString<32> clang::getIssueHash(const FullSourceLoc &IssueLoc,
+                                    StringRef CheckerName,
+                                    StringRef WarningMessage,
+                                    const Decl *IssueDecl,
                                     const LangOptions &LangOpts) {
 
-  return GetHashOfContent(
-      GetIssueString(SM, IssueLoc, CheckerName, BugType, D, LangOpts));
+  return GetMD5HashOfContent(getIssueString(
+      IssueLoc, CheckerName, WarningMessage, IssueDecl, LangOpts));
 }

clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

 //==- ExprInspectionChecker.cpp - Used for regression tests ------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "Taint.h"
+#include "clang/Analysis/IssueHash.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/SValExplainer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 using namespace clang;
 using namespace ento;

 }
 
 void ExprInspectionChecker::analyzerHashDump(const CallExpr *CE,
                                              CheckerContext &C) const {
   const LangOptions &Opts = C.getLangOpts();
   const SourceManager &SM = C.getSourceManager();
   FullSourceLoc FL(CE->getArg(0)->getBeginLoc(), SM);
   std::string HashContent =
-      GetIssueString(SM, FL, getCheckerName().getName(), "Category",
+      getIssueString(FL, getCheckerName().getName(), "Category",
                      C.getLocationContext()->getDecl(), Opts);
 
   reportBug(HashContent, C);
 }
 
 void ExprInspectionChecker::analyzerDenote(const CallExpr *CE,
                                            CheckerContext &C) const {
   if (CE->getNumArgs() < 2) {

clang/lib/StaticAnalyzer/Core/CMakeLists.txt

add_clang_library(clangStaticAnalyzerCore
   ExplodedGraph.cpp
   ExprEngine.cpp
   ExprEngineC.cpp
   ExprEngineCXX.cpp
   ExprEngineCallAndReturn.cpp
   ExprEngineObjC.cpp
   FunctionSummary.cpp
   HTMLDiagnostics.cpp
-  IssueHash.cpp
   LoopUnrolling.cpp
   LoopWidening.cpp
   MemRegion.cpp
   PlistDiagnostics.cpp
   ProgramState.cpp
   RangeConstraintManager.cpp
   RangedConstraintManager.cpp
   RegionStore.cpp

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

 //===- HTMLDiagnostics.cpp - HTML Diagnostics for Paths -------------------===//
 //
 // 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 HTMLDiagnostics object.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Analysis/IssueHash.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
// Embed meta-data tags.
 
     os << "\n<!-- BUGFILE " << DirName << Entry->getName() << " -->\n";
 
     os << "\n<!-- FILENAME " << llvm::sys::path::filename(Entry->getName()) << " -->\n";
 
     os  << "\n<!-- FUNCTIONNAME " <<  declName << " -->\n";
 
     os << "\n<!-- ISSUEHASHCONTENTOFLINEINCONTEXT "
-       << GetIssueHash(SMgr, L, D.getCheckerName(), D.getBugType(),
-                       DeclWithIssue, PP.getLangOpts())
+       << getIssueHash(L, D.getCheckerName(), D.getBugType(), DeclWithIssue,
+                       PP.getLangOpts())
        << " -->\n";
 
     os << "\n<!-- BUGLINE "
        << LineNumber
        << " -->\n";
 
     os << "\n<!-- BUGCOLUMN "
       << ColumnNumber

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

 //===--- PlistDiagnostics.cpp - Plist 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 PlistDiagnostics object.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Analysis/IssueHash.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/PlistSupport.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/TokenConcatenation.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/Casting.h"
 #include <memory>
 
 using namespace clang;
for (std::vector<const PathDiagnostic*>::iterator DI=Diags.begin(),
     o << "   <!-- This hash is experimental and going to change! -->\n";
     o << "   <key>issue_hash_content_of_line_in_context</key>";
     PathDiagnosticLocation UPDLoc = D->getUniqueingLoc();
     FullSourceLoc L(SM.getExpansionLoc(UPDLoc.isValid()
                                             ? UPDLoc.asLocation()
                                             : D->getLocation().asLocation()),
                     SM);
     const Decl *DeclWithIssue = D->getDeclWithIssue();
-    EmitString(o, GetIssueHash(SM, L, D->getCheckerName(), D->getBugType(),
+    EmitString(o, getIssueHash(L, D->getCheckerName(), D->getBugType(),
                                DeclWithIssue, LangOpts))
         << '\n';
 
     // Output information about the semantic context where
     // the issue occurred.
     if (const Decl *DeclWithIssue = D->getDeclWithIssue()) {
       // FIXME: handle blocks, which have no name.
       if (const NamedDecl *ND = dyn_cast<NamedDecl>(DeclWithIssue)) {