sbarzowski added inline comments.

================
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 {
----------------
sbarzowski wrote:
> vmiklos wrote:
> > Prazek wrote:
> > > 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?
> > LibreOffice might be a candidate, see 
> > <https://wiki.documentfoundation.org/Development/ClangTidy> for details on 
> > how to generate a compile database for it (since it does not use cmake), 
> > then you can start testing.
> Thanks. However it's not just about using exception. To be meaningful I need 
> a project that use noexcept in more than a few places.
> 
> Here are some projects that I found:
> https://github.com/facebook/hhvm/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/folly/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/Tencent/mars/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/rocksdb/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/CRYTEK-CRYENGINE/CRYENGINE/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/SFTtech/openage/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/watchman/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/proxygen/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/philsquared/Catch/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/sandstorm-io/capnproto/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/miloyip/rapidjson/search?utf8=%E2%9C%93&q=noexcept
> 
> I've tried HHVM so far, and ran into some problems compiling it. Anyway I'm 
> working on it.
Ok, I was able to run it on most of the HHVM  codebase + deps. There were some 
issues that looked like some autogenerated pieces missing, so it may have been 
not 100% covered.

The results:
3 occurences in total
1) rethrow in destructor (http://pastebin.com/JRNMZGev)
2) false positive "throw and catch in the same function" 
(http://pastebin.com/14y1AJgM)
3) noexcept decided during compilation (http://pastebin.com/1jZzRAbC)

That's not too many, but this is a kind of check that I guess would be most 
useful earlier in the development - ideally before the initial code review.

I'm not sure if we should count (3) as false positive. We could potentially 
eliminate it, by checking if the expression in noexcept is empty or true 
literal.

(2) is def. a false positive. The potential handling of this case was already 
proposed, but I'm not sure if it's worth it.  The code in example (2) is ugly 
and extracting these throwing parts to separate functions looks like a good way 
to start refactoring. 

What do you think?



https://reviews.llvm.org/D19201



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to