[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

baloghadamsoftware added a comment. I tested the latest revision (this fronted patch already included) on my test file in https://reviews.llvm.org/D33537. Disregarding of the (not so important) check-specific parameters (`EnabledFunctions` and `IgnoredExceptions`) I do not get warnings for any

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

sberg added a comment. see also https://reviews.llvm.org/rL306715 "Fixed -Wexceptions derived-to-base false positives" Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

This revision was automatically updated to reflect the committed changes. Closed by commit rL306149: Emit warning when throw exception in destruct or dealloc functions which has a (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D3?vs=103519=103765#toc

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a few small nits with the test cases, this LGTM! You should hold off on committing for a day or two in case Richard or others have comments on the patch.

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 102877. jyu2 added a comment. update patch https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/AnalysisBasedWarnings.cpp test/CXX/except/except.spec/p11.cpp test/SemaCXX/warn-throw-out-noexcept-func.cpp Index:

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 added inline comments. Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:27 +} + +struct N : A { aaron.ballman wrote: > jyu2 wrote: > > aaron.ballman wrote: > > > Can you add a test case like: > > > ``` > > > struct Throws { > > > ~Throws()

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added inline comments. Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:27 +} + +struct N : A { jyu2 wrote: > aaron.ballman wrote: > > Can you add a test case like: > > ``` > > struct Throws { > > ~Throws() noexcept(false); > > }; > > >

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 102868. jyu2 added a comment. Update patch https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/AnalysisBasedWarnings.cpp test/CXX/except/except.spec/p11.cpp test/SemaCXX/warn-throw-out-noexcept-func.cpp Index:

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 added inline comments. Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:1 +// RUN: %clang_cc1 %s -fdelayed-template-parsing -fcxx-exceptions -fexceptions -fsyntax-only -Wexceptions -verify -std=c++11 +struct A { aaron.ballman wrote: >

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added inline comments. Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:1 +// RUN: %clang_cc1 %s -fdelayed-template-parsing -fcxx-exceptions -fexceptions -fsyntax-only -Wexceptions -verify -std=c++11 +struct A { rnk wrote: >

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

rnk added inline comments. Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:1 +// RUN: %clang_cc1 %s -fdelayed-template-parsing -fcxx-exceptions -fexceptions -fsyntax-only -Wexceptions -verify -std=c++11 +struct A { aaron.ballman wrote: > I believe

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added a comment. A few more nits, but this feels like it's getting close to ready (at least, to me). Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:1 +// RUN: %clang_cc1 %s -fdelayed-template-parsing -fcxx-exceptions -fexceptions -fsyntax-only

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 102828. jyu2 added a comment. Thanks Aaron!!! I just upload new patch to address your comments. I now understand your point on when I can use auto. https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:304 + ->getUnqualifiedDesugaredType(); + if (CaughtType == nullptr || CaughtType == ThrowType) +return true; aaron.ballman wrote: > The check for a null

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 marked 7 inline comments as done. jyu2 added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:296 + + if (ThrowType->isReferenceType()) +ThrowType = ThrowType->castAs() aaron.ballman wrote: > If `ThrowType` can be null, there should be a

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 102759. jyu2 marked 13 inline comments as done. https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/AnalysisBasedWarnings.cpp test/CXX/except/except.spec/p11.cpp test/SemaCXX/warn-throw-out-noexcept-func.cpp

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 102754. jyu2 added a comment. Address Aaron's comments. https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/AnalysisBasedWarnings.cpp test/CXX/except/except.spec/p11.cpp

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Adding Richard to the review for some wider perspective than just mine on the overall design. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:296 + + if (ThrowType->isReferenceType()) +ThrowType =

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 101373. jyu2 added a comment. Add more test include 1> throw/catch reference types. 2> try block. 3>unreachable code. https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/AnalysisBasedWarnings.cpp

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 marked 2 inline comments as done. jyu2 added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:334 + continue; +else + HasThrowOutFunc = true; jyu2 wrote: > aaron.ballman wrote: > > You can drop the `else` here and just set

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 marked 12 inline comments as done. jyu2 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6341 +: Warning<"%0 has a non-throwing exception specification but can still " + "throw, may result in unexpected program termination.">, +

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 added a comment. In https://reviews.llvm.org/D3#770238, @aaron.ballman wrote: > In https://reviews.llvm.org/D3#768332, @jyu2 wrote: > > > Okay this CFG version of this change. In this change I am basic using same > > algorithm with -Winfinite-recursion. > > > > In addition to my

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 marked 8 inline comments as done. jyu2 added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:334 + continue; +else + HasThrowOutFunc = true; aaron.ballman wrote: > You can drop the `else` here and just set `HasThrowOutFunc` to

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 101167. jyu2 marked an inline comment as done. jyu2 added a comment. Update to address review comments. https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/AnalysisBasedWarnings.cpp

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added a comment. In https://reviews.llvm.org/D3#768332, @jyu2 wrote: > Okay this CFG version of this change. In this change I am basic using same > algorithm with -Winfinite-recursion. > > In addition to my original implementation, I add handler type checking which > basic

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 100796. jyu2 added a comment. Okay this CFG version of this change. In this change I am basic using same algorithm with -Winfinite-recursion. In addition to my original implementation, I add handler type checking which basic using

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 added a comment. In https://reviews.llvm.org/D3#760431, @aaron.ballman wrote: > In https://reviews.llvm.org/D3#760419, @jyu2 wrote: > > > In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote: > > > > > As an FYI, there is a related check currently under development in >

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. In https://reviews.llvm.org/D3#760419, @jyu2 wrote: > In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote: > > > As an FYI, there is a related check currently under development in > > clang-tidy; we

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 added a comment. In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote: > As an FYI, there is a related check currently under development in > clang-tidy; we probably should not duplicate this functionality in both > places. See https://reviews.llvm.org/D19201 for the other

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

Prazek added a comment. Could you add similar tests as the ones that Stanislaw provied in his patch? Like the one with checking if throw is catched, or the conditional noexcept (by a macro, etc) https://reviews.llvm.org/D3 ___ cfe-commits

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added a comment. As an FYI, there is a related check currently under development in clang-tidy; we probably should not duplicate this functionality in both places. See https://reviews.llvm.org/D19201 for the other review. https://reviews.llvm.org/D3

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 updated this revision to Diff 99517. jyu2 added a comment. Reid, Thank you so much for your comments. I upload new patch to address your suggestion. 1> Emit warning for throw exception in all noexcept function. And special diagnostic note for destructor and delete operators. 2> Silence

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

lebedev.ri added a comment. Relevant https://reviews.llvm.org/D31370, https://reviews.llvm.org/D19201 https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

rnk added a comment. I think we should consider generalizing this to throwing from any noexcept function. We could add a special case diagnostic to explain that destructors and delete operators are noexcept by default in C++11. It's also probably a good idea to silence this warning if there

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

jyu2 created this revision. Throwing in the destructor is not good (C++11 change try to not allow see below). But in reality, those codes are exist. C++11 [class.dtor]p3: A declaration of a destructor that does not have an exception-specification is implicitly considered to have the same