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); ---------------- alexfh wrote: > Prazek wrote: > > 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. > The `FIX-IT applied suggested code changes` notes are shown by clang-tidy > itself and only when the user passes -fix flag. There's no way to control > them from the check (apart from removing fixes or attaching them to a > different warning). That's right, but does it make sense to show user all the places where the function was declared? I am not sure what clang would normally do ================ Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20 +///\brief Warns about using throw in function declared as noexcept. +/// It complains about every throw, even if it is caught later. +class ThrowWithNoexceptCheck : public ClangTidyCheck { ---------------- aaron.ballman wrote: > What is the false positive rate with this check over a large codebase that > uses exceptions? Do you know any good project to check it? https://reviews.llvm.org/D19201 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits