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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to