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.
================
Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:5-6
+}; // implicitly noexcept(true)
+A_ShouldDiag::~A_ShouldDiag() { // expected-note {{destructor or deallocator
has a}}
+ throw 1; // expected-warning {{has a non-throwing exception specification
but}}
+}
----------------
The first test case showing a diagnostic or a note in the test should spell out
the full text of the diagnostics. The remainder of the diagnostic checks can be
shortened somewhat, but this ensures the full diagnostic text is properly
checked.
================
Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:8-11
+struct B_ShouldDiag {
+ int i;
+ ~B_ShouldDiag() noexcept(true) {}
+};
----------------
Why name this ShouldDiag if it doesn't diagnose?
================
Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:126
+struct Throws {
+ ~Throws()noexcept(false);
+};
----------------
Add a space before the `noexcept`.
================
Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:131
+ Throws T;
+ ~ShouldDiagnose() noexcept{ //expected-note {{destructor or deallocator has
a}}
+ throw; // expected-warning {{has a non-throwing exception specification
but}}
----------------
Add a space before the `{`.
Repository:
rL LLVM
https://reviews.llvm.org/D33333
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
