[PATCH] D10305: [Clang Static Analyzer] Bug identification

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

> Generating mangled names requires ASTContext which is not available during the error reporting. BugReporter does have the ASTContext, so it would not 

>  be a big change to add it to the DiagnosticConsumers though. And I think the mangled name might contain too much redundant information. The only 

>  relevant information here is the fully qualified name and the parts of the signature that can be ocerloaded on e.g.: constness. Using this method might also 

>  be easier to debug, since the IssueString is more readable.


I think it'd be fine to pass ASTContext to the DiagnosticConsumers. It would be useful to find out exactly what extra information the mangled names have that we do not want to see in the issue_hash.

One issue that I see with the current approach is that we do not differentiate methods from different classes/namespaces. Is this by design?

> I definitely agree. It would be great to have a unique identifier for checkers that is independent from the name/package. It is not a trivial problem however,

>  since we probably want to support plugins too. I can think of a similar solution like git commit ids, but I think this problem might be out of the scope of this

>  patch. But I am happy to start a discussion on the mailing list and create a new patch to solve this.


Sounds good to me. I agree that this is out of scope for this patch.


================
Comment at: include/clang/StaticAnalyzer/Core/IssueHash.h:32
@@ +31,3 @@
+llvm::SmallString<32> GetIssueHash(const SourceManager &SM,
+                                   FullSourceLoc &UniqueLoc,
+                                   llvm::StringRef CheckerName,
----------------
UniqueLoc -> IssueLoc ?

================
Comment at: utils/analyzer/CmpRuns.py:74
@@ -73,3 +73,3 @@
         if 'issue_hash' in self._data :
-          id += str(self._data['issue_hash'])
+          id += str(self._data['issue_hash_function_offset'])
         return id
----------------
We should use the new hash here.


http://reviews.llvm.org/D10305