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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
