[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation
Gábor Horváth via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 11 04:45:32 PDT 2017
xazax.hun added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/IssueHash.cpp:39
+ // primary template.
+ if (const FunctionDecl *InstantiatedFrom =
+ Target->getInstantiatedFromMemberFunction())
----------------
martong wrote:
> Could we use here FunctionDecl::getPrimaryTemplate() ? That seems more general, it handles both specializations and instantiations.
Unfortunately `getPrimaryTemplate` is not sufficient. The function might be a member function of a template class. In this case, there is no primary template for the function (only for the enclosing class) but it still depends on a template parameter.
================
Comment at: test/Analysis/bug_hash_test.cpp:61
-// CHECK: <key>diagnostics</key>
+template <typename T>
+T f(T i) {
----------------
martong wrote:
> We could add a few more test cases:
> - a function template in class template
> - specializations vs instantiations
> - the combination of the above two (?)
>
Good point.
================
Comment at: test/Analysis/bug_hash_test.cpp:1363
// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>path</key>
----------------
martong wrote:
> I am not sure if this is possible, but could we add unit test just for the `GetSignature` function? Instead of these huge plist files?
>
> I am thinking something like this:
> https://github.com/martong/friend-stats/blob/ed0c69ea3669c933204c799f59b85cd7b2507c34/ut/FriendFunctionsTest.cpp#L31
I think it is more convenient to use regression test for this purpose than unittests. But I replaced the long plist checking with something much more concise.
https://reviews.llvm.org/D38728
More information about the cfe-commits
mailing list