[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Thank you for working on this!
I just tried, and the original false-positive i was hitting is now gone.

So as far i'm concerned, this is good to go.



================
Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:26
   }
   throw non_derived_exception(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
----------------
JonasToth wrote:
> lebedev.ri wrote:
> > Could you please update the comments?
> > I find them misleading. What does this `bad` mean?
> > That this line is not handled correctly? Then please use `FIXME: wrong handling`
> > That this line is invalid? The `// CHECK-MESSAGES:` already states that...
> yes, they are left over when i started writing the test and did not know the diagnostics. These comments help, when something is reported from clang-tidy, to instantly see if this is correctly found, since the source line is shown. I can remove them.
> These comments help, when something is reported from clang-tidy, to instantly see if this is correctly found, since the source line is shown. I can remove them.

Aha. I'd use some less confusing comments then, maybe


================
Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:54-62
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
----------------
JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > these diagnostic come from the many instantiations of this function. 
> > > do you think, they should exist or rather not?
> > They definitively should exist.
> > But they also should ideally point to the origin of the `T`.
> they kinda do. when templates are used, the template class is pointed to, but not the instantiation. At least they shouldn't point to the `<typename T>` anymore.
> but not the instantiation 
It would be best if they would, but looking at the AST, i'm not sure how to do that...
https://godbolt.org/g/ejScyZ

Maybe someone else knows.


https://reviews.llvm.org/D37060