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

[Aaron Ballman via Phabricator via cfe-commits]

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