[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.


================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54
+    // FIXME use DiagnosticIDs::Level::Note
+    diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note)
+        << FixItHint::CreateRemoval(NoExceptRange);
----------------
sbarzowski wrote:
> Prazek wrote:
> > sbarzowski wrote:
> > > alexfh wrote:
> > > > nit: `nothrow` (without a dash), no colon needed (it will look weird, since the location is mentioned _before_ the message, not after it)
> > > No, it's after the message now. When I changed the level to note the order of messages changed as well.
> > > 
> > > It looks like that:
> > > ```
> > > /Users/uland/clang-new/build/tools/clang/tools/extra/test/clang-tidy/Output/misc-throw-with-noexcept.cpp.tmp.cpp:5:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
> > >     throw 5;
> > >     ^
> > > /Users/uland/clang-new/build/tools/clang/tools/extra/test/clang-tidy/Output/misc-throw-with-noexcept.cpp.tmp.cpp:3:24: note: FIX-IT applied suggested code changes
> > > void f_throw_with_ne() noexcept(true) {
> > >                        ^
> > > /Users/uland/clang-new/build/tools/clang/tools/extra/test/clang-tidy/Output/misc-throw-with-noexcept.cpp.tmp.cpp:3:24: note: in a function declared nothrow here:
> > > void f_throw_with_ne() noexcept(true) {
> > >                        ^
> > > 
> > > ```
> > > 
> > > So, should I leave the colon or remove it anyway?
> > I think that the best way would be to have warnings in order:
> > 
> > warning function declared nothrow here have throw statement inside:
> > Note: throw statement here
> > 
> > Note: Fixit applied for every other declaration
> > 
> > What do you think Alex?
> > 
> > 
> > 
> @Prazek 
> So, do you suggest that we don't emit anything for additional declarations (without -fix)?
> 
> BTW (in the current version) I don't know if I can control if FIX-IT goes first or the location message. As you can see in the code the FIX-IT goes after the location.
Yep, there is no need for user to know all the locations if he doesn't want to perform fixit. This way it is easier to read the warning.


https://reviews.llvm.org/D19201