JonasToth added inline comments.
================ Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception' + ---------------- JonasToth wrote: > JonasToth wrote: > > alexfh wrote: > > > hokein wrote: > > > > I think giving message on the template function here is confusing to > > > > users even it gets instantiated somewhere in this TU -- because users > > > > have to find the location that triggers the template instantiation. > > > > > > > > Maybe > > > > 1) Add a note which gives the instantiation location to the message, or > > > > 2) ignore template case (some clang-tidy checks do this) > > > In this particular case it seems to be useful to get warnings from > > > template instantiations. But the message will indeed be confusing. > > > > > > Ideally, the message should have "in instantiation of xxx requested here" > > > notes attached, as clang warnings do. But this is not working > > > automatically, and it's implemented in Sema > > > (`Sema::PrintInstantiationStack()` in > > > lib/Sema/SemaTemplateInstantiate.cpp). > > > > > > I wonder whether it's feasible to produce similar notes after Sema is > > > dead? Maybe not the whole instantiation stack, but at least it should be > > > possible to figure out that the enclosing function is a template > > > instantiation or is a member function of an type that is an instantiation > > > of a template. That would be useful for other checks as well. > > It should be possible to figure out, that the type comes from template > > instantiation and that information could be added to the warning. > > > > I will take a look at Sema and think about something like this. > > Unfortunatly i dont have a lot of time :/ > I did look further into the issue, i think it is non-trivial. > > The newly added case is not a templated exception perse, but there is a > exception-factory, which is templated, that creates a normal exception. > > I did add another note for template instantiations, but i could not figure > out a way to give better diagnostics for the new use-case. @hokein and @alexfh Do you still have your concerns (the exception is not a template value, but the factory creating them) or is this fix acceptable? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits