aaron.ballman 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:
> 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?
>
The fact that there's a false positive in the first large project you checked
suggests that the false positive is likely worth it to fix. The code may be
ugly, but it's not uncommon -- some coding guidelines explicitly disallow use
of gotos, and this is a reasonable(ish) workaround for that.
As for #3, I would consider that to be a false-positive as well. A computed
noexcept clause is going to be very common, especially in generic code. I think
it's probably more important to get this one right than #2.
https://reviews.llvm.org/D19201
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
