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

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

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



================
Comment at: clang/lib/Analysis/IssueHash.cpp:183
 
-std::string clang::GetIssueString(const SourceManager &SM,
-                                  FullSourceLoc &IssueLoc,
-                                  StringRef CheckerName, StringRef BugType,
-                                  const Decl *D,
-                                  const LangOptions &LangOpts) {
+std::string clang::getIssueStringV1(const FullSourceLoc &IssueLoc,
+                                    StringRef CheckerName,
----------------
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?


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

https://reviews.llvm.org/D67421