[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang/include/clang/Analysis/IssueHash.h:18
 
 /// Get an MD5 hash to help identify bugs.
 ///
----------------
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,
- ...


================
Comment at: clang/include/clang/Analysis/IssueHash.h:35
+///
 /// 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
----------------
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?


================
Comment at: clang/include/clang/Analysis/IssueHash.h:41
+llvm::SmallString<32>
+GetIssueHash(FullSourceLoc &IssueLoc,
+             llvm::StringRef CheckerName, llvm::StringRef WarningMessage,
----------------
Why isn't `IssueLoc` const?


================
Comment at: clang/include/clang/Analysis/IssueHash.h:47
 /// more information.
-std::string GetIssueString(const SourceManager &SM, FullSourceLoc &IssueLoc,
-                           llvm::StringRef CheckerName, llvm::StringRef BugType,
-                           const Decl *D, const LangOptions &LangOpts);
+std::string GetIssueString(FullSourceLoc &IssueLoc,
+                           llvm::StringRef CheckerName,
----------------
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67421/new/

https://reviews.llvm.org/D67421