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

Reply via email to