alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed.
================ Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:6 + +Finds functions which should not throw exceptions: +* Destructors ---------------- I don't think the check just finds functions that should not throw exceptions. I suppose, it finds `throw` statements inside them (or maybe it goes slightly deeper - the documentation should explain this in more detail). See also the comment @dberris left below. ================ Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:31 + ``WinMain()`` in the Windows API to the list of the funcions which should + not throw.. Default value is empty string. + ---------------- nit: Double period after `throw`. ================ Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:31 + ``WinMain()`` in the Windows API to the list of the funcions which should + not throw.. Default value is empty string. + ---------------- alexfh wrote: > nit: Double period after `throw`. nit: "an empty string" Same below. ================ Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178 +void indirect_implicit() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws + implicit_int_thrower(); ---------------- baloghadamsoftware wrote: > lebedev.ri wrote: > > baloghadamsoftware wrote: > > > dberris wrote: > > > > How deep does this go? Say we have a call to a function that's extern > > > > which doesn't have 'noexcept' nor a dynamic exception specifier -- do > > > > we assume that the call to an extern function may throw? Does that > > > > warn? What does the warning look like? Should it warn? How about when > > > > you call a function through a function pointer? > > > > > > > > The documentation should cover these cases and/or more explicitly say > > > > in the warning that an exception may throw in a noexcept function > > > > (rather than just "function <...> throws"). > > > We take the most conservative way here. We assume that a function throws > > > if and only if its body has a throw statement or its header has the (old) > > > throw() exception specification. We do not follow function pointers. > > While i realize it may have more noise, it may be nice to have a more > > pedantic mode (guarded by an option?). > > E.g. `noexcept` propagation, much like `const` on methods propagation. > > Or at least a test that shows that it does not happen, unless i simply did > > not notice it :) > This could be an enhancement in the future, yes. Please address the original comment here. In particular, the warning message should be clearer (that an exception may be thrown in a function that should not throw exceptions). https://reviews.llvm.org/D33537 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits