Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

[jyu2]

Summary

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 exception specification as an implicit declaration.

With this change, the application worked before may now run into runtime termination. May gold here is to emit a warning to provide only possible info to where the code may need to be changed.

First there is no way, in compile time to identify the “throw” really throw out of the function. Things like the call which throw out… To keep this simple, when “throw” is seen, checking its enclosing function(only destructor and dealloc functions) with noexcept(true) specifier emit warning.

Here is implementation detail:
A new member function CheckCXXThrowInNonThrowingFunc is added for class Sema in Sema.h. It is used in the call to both BuildCXXThrow and TransformCXXThrowExpr.

The function basic check if the enclosing function with non-throwing noexcept specifer, if so emit warning for it.

The example of warning message like:
k1.cpp:18:3: warning: ''~dependent_warn'' has a (possible implicit) non-throwing

    noexcept specifier. Throwing exception may cause termination.
    [-Wthrow-in-dtor]
throw 1;
^

k1.cpp:43:30: note: in instantiation of member function

'dependent_warn<noexcept_fun>::~dependent_warn' requested here

dependent_warn<noexcept_fun> f; // cause warning

Let me know if more information is needed.

Thanks.

Jennifer

[rnk]

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 are any try scopes around the exception being thrown.

[lebedev.ri]

Relevant D31370, D19201

[jyu2]

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 this warning when the throw inside try block.

Let me know if more information is needed.

Thanks.

Jennifer

[aaron.ballman]

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.

[Prazek]

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)

[jyu2]

In D33333#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 review.

To my understanding, clang-tidy is kind of source check tool. It does more intensive checking than compiler.
My purpose here is to emit minimum warning form compiler, in case, clang-tidy is not used.

In D33333#760353, @Prazek wrote:

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)

Good idea! Could add “marco” test for this. But I am not sure compiler want to add check for throw caught by different catch handler. Because at compile time, compiler can not statically determine which catch handler will be used to caught the exception on all time. I would think that is pragma's responsibility.

For example:

If (a) throw A else throw B;

My main concern there is implicit noexcept gets set by compiler, which cause runtime to termination.

[aaron.ballman]

In D33333#760419, @jyu2 wrote:

In D33333#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 review.

To my understanding, clang-tidy is kind of source check tool. It does more intensive checking than compiler.
My purpose here is to emit minimum warning form compiler, in case, clang-tidy is not used.

Sort of correct. clang-tidy doesn't necessarily do more intensive checking than the compiler. It's more an AST matching source checking tool for checks that are either expensive or have a higher false-positive rate than we'd want to see in the frontend.

I think that this particular check should probably be in the frontend, like you're doing. My biggest concern is that we do not duplicate functionality between the two tools. D19201 has has a considerable amount of review, so you should look at the test cases there and ensure you are handling them (including the fixits). Hopefully your patch ends up covering all of the functionality in the clang-tidy patch.

In D33333#760353, @Prazek wrote:

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)

Good idea! Could add “marco” test for this. But I am not sure compiler want to add check for throw caught by different catch handler. Because at compile time, compiler can not statically determine which catch handler will be used to caught the exception on all time. I would think that is pragma's responsibility.

For example:

If (a) throw A else throw B;

My main concern there is implicit noexcept gets set by compiler, which cause runtime to termination.

[jyu2]

In D33333#760431, @aaron.ballman wrote:

In D33333#760419, @jyu2 wrote:

In D33333#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 review.

To my understanding, clang-tidy is kind of source check tool. It does more intensive checking than compiler.
My purpose here is to emit minimum warning form compiler, in case, clang-tidy is not used.

Sort of correct. clang-tidy doesn't necessarily do more intensive checking than the compiler. It's more an AST matching source checking tool for checks that are either expensive or have a higher false-positive rate than we'd want to see in the frontend.

I think that this particular check should probably be in the frontend, like you're doing. My biggest concern is that we do not duplicate functionality between the two tools. D19201 has has a considerable amount of review, so you should look at the test cases there and ensure you are handling them (including the fixits). Hopefully your patch ends up covering all of the functionality in the clang-tidy patch.

In D33333#760353, @Prazek wrote:

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)

Good idea! Could add “marco” test for this. But I am not sure compiler want to add check for throw caught by different catch handler. Because at compile time, compiler can not statically determine which catch handler will be used to caught the exception on all time. I would think that is pragma's responsibility.

For example:

If (a) throw A else throw B;

My main concern there is implicit noexcept gets set by compiler, which cause runtime to termination.

As I said, I don't think checking throw type matching catch handler type is compiler's job. I'd like either silence all warning for that. What do you think?

[aaron.ballman]

In D33333#761126, @jyu2 wrote:

In D33333#760431, @aaron.ballman wrote:

In D33333#760419, @jyu2 wrote:

In D33333#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 review.

To my understanding, clang-tidy is kind of source check tool. It does more intensive checking than compiler.
My purpose here is to emit minimum warning form compiler, in case, clang-tidy is not used.

Sort of correct. clang-tidy doesn't necessarily do more intensive checking than the compiler. It's more an AST matching source checking tool for checks that are either expensive or have a higher false-positive rate than we'd want to see in the frontend.

I think that this particular check should probably be in the frontend, like you're doing. My biggest concern is that we do not duplicate functionality between the two tools. D19201 has has a considerable amount of review, so you should look at the test cases there and ensure you are handling them (including the fixits). Hopefully your patch ends up covering all of the functionality in the clang-tidy patch.

In D33333#760353, @Prazek wrote:

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)

Good idea! Could add “marco” test for this. But I am not sure compiler want to add check for throw caught by different catch handler. Because at compile time, compiler can not statically determine which catch handler will be used to caught the exception on all time. I would think that is pragma's responsibility.

For example:

If (a) throw A else throw B;

My main concern there is implicit noexcept gets set by compiler, which cause runtime to termination.

As I said, I don't think checking throw type matching catch handler type is compiler's job. I'd like either silence all warning for that. What do you think?

Consider the following cases:

void f() noexcept { // Should not diagnose
  try {
    throw 12;
  } catch (int) {
  }
}

void g() noexcept { // Should not diagnose
  try {
    throw 12;
  } catch (...) {
  }
}

void h() noexcept { // Should diagnose
  try {
    throw 12;
  } catch (const char *) {
  }
}

void i() noexcept { // Should diagnose
  try {
    throw 12;
  } catch (int) {
    throw;
  }
}

void j() noexcept { // Should diagnose
  try {
    throw 12;
  } catch (int) {
    throw "haha";
  }
}

void k() noexcept { // Should diagnose
  try {
    throw 12;
  } catch (...) {
    throw;
  }
}

I think the expected behavior in these cases is reasonable (and can be done with a CFG) rather than manually looking at the scope stack like you're doing.

include/clang/Basic/DiagnosticGroups.td

140 ↗

(On Diff #99517)

No need to add a group for this; you can define one inline with the warning, but perhaps the exceptions group already covers it.

include/clang/Basic/DiagnosticSemaKinds.td

6345

Do not quote %0. Also, remove the full-stop and capitalized "Throwing". Diagnostics are not complete sentences. I think it should probably read "%0 has a non-throwing exception specification but can still throw, resulting in unexpected program termination" (or something along those lines).

6352

Rather than make the note spell out all the various ways a function can be nothrow, we should either say "nonthrowing function declare here" or specify which reason the function is not throwing using %select.

include/clang/Sema/Sema.h

4970 ↗

(On Diff #99517)

with non-throwing -> with a non-throwing

lib/Sema/SemaExprCXX.cpp

690 ↗

(On Diff #99517)

Please run clang-format over the patch (the asterisk binds to FD and the curly brace should be on this line).

692 ↗

(On Diff #99517)

Can use const auto * here.

693–696 ↗

(On Diff #99517)

Is there a reason for doing this rather than simply calling FunctionProtoType::isNothrow()?

712 ↗

(On Diff #99517)

Please spell out the type explicitly here.

713 ↗

(On Diff #99517)

Remove spurious parens.

716 ↗

(On Diff #99517)

Same here.

test/CXX/except/except.spec/p11.cpp

3–4

Please modify this test to expect diagnostics rather than remove the noexcept from the test. This is testing specific features of the standard.

test/SemaCXX/warn-throw-out-dtor.cpp

1 ↗

(On Diff #99517)

Please remove the svn props from this file.

[rnk]

Re: clang-tidy, I would rather implement this as a traditional compiler warning.

In D33333#761208, @aaron.ballman wrote:

In D33333#761126, @jyu2 wrote:

As I said, I don't think checking throw type matching catch handler type is compiler's job. I'd like either silence all warning for that. What do you think?

Consider the following cases:
...
I think the expected behavior in these cases is reasonable (and can be done with a CFG) rather than manually looking at the scope stack like you're doing.

I agree with @jyu2, we should do something simple. I think it would be simple to handle all cases except for h, which requires semantic analysis to figure out if the thrown object would be caught by the catch parameter. Implementing that is more likely to introduce bugs in the compiler than it is to find bugs in user code. I doubt we need the CFG for any of this. The later examples are all throwing exceptions from catch blocks, which is the same as not having an open try scope.

[aaron.ballman]

In D33333#761224, @rnk wrote:

Re: clang-tidy, I would rather implement this as a traditional compiler warning.

In D33333#761208, @aaron.ballman wrote:

In D33333#761126, @jyu2 wrote:

As I said, I don't think checking throw type matching catch handler type is compiler's job. I'd like either silence all warning for that. What do you think?

Consider the following cases:
...
I think the expected behavior in these cases is reasonable (and can be done with a CFG) rather than manually looking at the scope stack like you're doing.

I agree with @jyu2, we should do something simple. I think it would be simple to handle all cases except for h, which requires semantic analysis to figure out if the thrown object would be caught by the catch parameter. Implementing that is more likely to introduce bugs in the compiler than it is to find bugs in user code.

I'm not certain I agree with the assertion it's more likely to introduce bugs in the compiler than find bugs in user code. Machine-generated code runs into silly things like this, and it's nice to warn the user about it. However, it may be reasonable to do that in a follow-up patch -- but that patch is likely to have a lot of churn since it's not really plausible to implement with the way @jyu2 has structured this patch. By making this an analysis-based warning instead, that would alleviate my concerns (but would likely require implementing the CFG approach anyway).

I doubt we need the CFG for any of this. The later examples are all throwing exceptions from catch blocks, which is the same as not having an open try scope.

Sure, if you cut out the cases that require a CFG, you don't need a CFG for it. :-P Use of a CFG would eliminate obvious false-positives such as h().

Also, there should be some tests with function-try-blocks.

[jyu2]

In D33333#761233, @aaron.ballman wrote:

In D33333#761224, @rnk wrote:

Re: clang-tidy, I would rather implement this as a traditional compiler warning.

In D33333#761208, @aaron.ballman wrote:

In D33333#761126, @jyu2 wrote:

As I said, I don't think checking throw type matching catch handler type is compiler's job. I'd like either silence all warning for that. What do you think?

Consider the following cases:
...
I think the expected behavior in these cases is reasonable (and can be done with a CFG) rather than manually looking at the scope stack like you're doing.

I agree with @jyu2, we should do something simple. I think it would be simple to handle all cases except for h, which requires semantic analysis to figure out if the thrown object would be caught by the catch parameter. Implementing that is more likely to introduce bugs in the compiler than it is to find bugs in user code.

I'm not certain I agree with the assertion it's more likely to introduce bugs in the compiler than find bugs in user code. Machine-generated code runs into silly things like this, and it's nice to warn the user about it. However, it may be reasonable to do that in a follow-up patch -- but that patch is likely to have a lot of churn since it's not really plausible to implement with the way @jyu2 has structured this patch. By making this an analysis-based warning instead, that would alleviate my concerns (but would likely require implementing the CFG approach anyway).

I doubt we need the CFG for any of this. The later examples are all throwing exceptions from catch blocks, which is the same as not having an open try scope.

Sure, if you cut out the cases that require a CFG, you don't need a CFG for it. :-P Use of a CFG would eliminate obvious false-positives such as h().

Also, there should be some tests with function-try-blocks.

Sure, I will add that try-block test for this.

I am agree with @rnk keep this simple.

[jyu2]

This is new version should address all @Aaron's commands, but CFG part.

I would not think we should go that far in the compiler for this.

Thanks.

[jyu2]

I missed two place which Aaron point out.
1> using isNothrow function instead NR_Nothrow.
2> A format problem.

Changed.
Thanks.

[jyu2]

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 https://reviews.llvm.org/D19201 method.

There are couple things I am worry about this implementation:
1> compile time...
2> Correctness...
3> Stack overflow for large CFG...

[aaron.ballman]

In D33333#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 using https://reviews.llvm.org/D19201 method.

Thank you, I think this is a step in the right direction!

There are couple things I am worry about this implementation:
1> compile time...

Do you have any timing data on whether this has a negative performance impact?

2> Correctness...

Your implementation looks reasonable to me, but with further review (and good tests), we should have a better grasp on correctness.

3> Stack overflow for large CFG...

I would be surprised if that were a problem, but is something we could address if it ever arises.

include/clang/Basic/DiagnosticSemaKinds.td

6346

throw, may -> throw, which may

Also, remove the period at the end of the diagnostic.

6349

possible -> possibly

6352

nonthrowing -> non-throwing

(to be consistent with other diagnostics.)

lib/Sema/AnalysisBasedWarnings.cpp

290

Caughted -> Caught

292

MayCaught -> IsCaught ?

293–294

Please don't use auto unless the type is explicitly spelled out in the initializer (here and elsewhere).

298

You can combine this with the above check.

304

This does not seem quite correct. Consider:

struct S{};

void f() {
  try {
    throw S{};
  } catch (const S *s) {
  }
}

311

mayThrowBeCaughtedByHandlers -> isThrowCaughtByAHandler

314

h -> H (or Idx, or anything else that meets the coding standard).

317–318

Just return true here instead of setting a local variable. Return false below.

330

Can use const auto * here.

[aaron.ballman]

lib/Sema/AnalysisBasedWarnings.cpp

324

Perhaps it's more clear as: doesThrowEscapePath()?

334

You can drop the else here and just set HasThrowOutFunc to true.

337

If OpLoc cannot be null, you should take it by reference. If it can be null, then you should guard against that here (and drop the parens).

338–340

You can use a range-based for loop over Block.succs()

376

Elide braces.

384

Can use range-based for loop over succs().

420

Elide braces.

2274

Space after //.
out in non-throwing -> out of a non-throwing

test/SemaCXX/warn-throw-out-noexcept-func.cpp

2

Please drop the svn auto props.

[jyu2]

Update to address review comments.

[jyu2]

lib/Sema/AnalysisBasedWarnings.cpp

334

Can not do that, don't if block has throw expression yet.

[jyu2]

In D33333#770238, @aaron.ballman wrote:

In D33333#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 using https://reviews.llvm.org/D19201 method.

Thank you, I think this is a step in the right direction!

There are couple things I am worry about this implementation:
1> compile time...

Do you have any timing data on whether this has a negative performance impact?

2> Correctness...

Your implementation looks reasonable to me, but with further review (and good tests), we should have a better grasp on correctness.

3> Stack overflow for large CFG...

I would be surprised if that were a problem, but is something we could address if it ever arises.

Hope that is okay.

[jyu2]

include/clang/Basic/DiagnosticSemaKinds.td

6346

use you suggested.

lib/Sema/AnalysisBasedWarnings.cpp

290

:-(

304

Good catch. Remove that check.

[jyu2]

lib/Sema/AnalysisBasedWarnings.cpp

334

Yes, Eric just point out, you are right, I can remove the line of "else"

[jyu2]

Add more test include 1> throw/catch reference types. 2> try block. 3>unreachable code.

[aaron.ballman]

Adding Richard to the review for some wider perspective than just mine on the overall design.

lib/Sema/AnalysisBasedWarnings.cpp

290

This should be isThrowCaught() (drop the Be).

292

Should be IsCaught by coding convention.

296

If ThrowType can be null, there should be a null pointer check here. If it cannot be null, you should use getTypePtr() above instead of getTypePtrOrNull().

304

The check for a null CaughtType can be hoisted above the if statements, which can then be simplified.

312

There's really no point to using a local variable for this. You can return true above and return false here.

315

isThrowCaughtByHandlers (drop the Be)

317

Can you hoist the getNumHandlers() call out? e.g., for (unsigned H = 0, E = TryStmt->getNumHandlers(); H < E; ++H)

318

No need for the local variable, can just call getHandler() in the call expression.

325

Since OpLoc cannot be null, pass it by reference instead.

330

This is marked as done but doesn't appear to have been done.

337

This should use const CFGBlock::AdjacentBlock & to avoid the copy, but could also use const auto & if you wanted (since it's a range-based for loop).

338

Instead of testing that I can be dereferenced (which is a bit odd since I is not a pointer, but uses a conversion operator), can you test I->isReachable() instead?

I think a better way to write this might be:

if (!I->isReachable())
  continue;

if (const CXXTryStmt *Terminator = dyn_cast_or_null<CXXTryStmt>(I->getTerminator())) {
}

344

There's no need for this local variable. You can return true here.

349

We don't usually mark locals as const unless it's a pointer or reference.

373

Same comment here as above.

374

Same comment here as above.

375

You can drop the parens around I. Also, next_ID should be something like NextID per the coding style guidelines.

405

cfg should be renamed per coding style guidelines. How about BodyCFG?

416

No need for the if statement since castAs<> will assert if the function type is not FunctionProtoType.

test/SemaCXX/warn-throw-out-noexcept-func.cpp

2

This does not appear to be done.

[jyu2]

Address Aaron's comments.

[jyu2]

lib/Sema/AnalysisBasedWarnings.cpp

296

Good catch. Add code and test to handle this

312

Right. Changed

315

removed "Be"

[aaron.ballman]

lib/Sema/AnalysisBasedWarnings.cpp

297

nit: !ThrowType

299

No need for testing ThrowType for null here.

304

This can still be hoisted higher in the function.

305

No need for testing CaughtType for null here.

332

const auto *

341

Oops, I should have suggested const auto * here with my original comment (sorry about that).

393

For consistency with other code in the compiler, can you move the Sema param to be the first parameter?

[jyu2]

Thanks Aaron!!! I just upload new patch to address your comments. I now understand your point on when I can use auto.

[aaron.ballman]

A few more nits, but this feels like it's getting close to ready (at least, to me).

test/SemaCXX/warn-throw-out-noexcept-func.cpp

2

This file still has the svn auto props.

2

I believe you can drop the -fcxx-exceptions as it should be implied by -fexceptions.

28

Can you add a test case like:

struct Throws {
  ~Throws() noexcept(false);
};

struct ShouldDiagnose {
  Throws T;
  ~ShouldDiagnose() {}
};

I would expect ~ShouldDiagnose() to be diagnosed as allowing exceptions to escape because of the destructor for Throws.

[rnk]

test/SemaCXX/warn-throw-out-noexcept-func.cpp

2

It isn't at the -cc1 level, you need -fcxx-exceptions there. -fexceptions controls landingpad cleanup emission.

[aaron.ballman]

test/SemaCXX/warn-throw-out-noexcept-func.cpp

2

Ah, thank you Reid, I forgot about that.

[jyu2]

test/SemaCXX/warn-throw-out-noexcept-func.cpp

2

I can remove -fexceptions, since I only care for syntax.

2

I am so sorry. Remove it.

28

In C++11, destructors are implicitly throw() unless any member or base of the type has a destructor with a different exception specification.

In the case of:
struct Throws {

~Throws() noexcept(false);

};

struct ShouldDiagnose {

Throws T;
~ShouldDiagnose() {}

};

You should not see diagnose for ~ShouldDiagnose() , since ShouldDiagnose has a member ofr Throws which has destructor with noexcept(false); therefor

~ShouldDiagnose has  noexcept(false).

But I add test case which remove (false) part.

[jyu2]

Update patch

[aaron.ballman]

test/SemaCXX/warn-throw-out-noexcept-func.cpp

28

Good point! A test case with noexcept(false) would be handy as would one where ~ShouldDiagnose() is marked noexcept(true) explicitly rather than picking up the noexcept(false) implicitly.

[jyu2]

update patch

test/SemaCXX/warn-throw-out-noexcept-func.cpp

28

Okay I add two tests ShouldDiagnoes and ShouldNotDiagnoes.

[jyu2]

Update test.

[aaron.ballman]

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.

test/SemaCXX/warn-throw-out-noexcept-func.cpp

6–7

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.

9–12

Why name this ShouldDiag if it doesn't diagnose?

127

Add a space before the noexcept.

132

Add a space before the {.

[sberg]

see also https://reviews.llvm.org/rL306715 "Fixed -Wexceptions derived-to-base false positives"

[baloghadamsoftware]

I tested the latest revision (this fronted patch already included) on my test file in D33537. Disregarding of the (not so important) check-specific parameters (EnabledFunctions and IgnoredExceptions) I do not get warnings for any indirect throws (indirect_implicit() and indirect_explicit()). So for me this frontend check does not seem to be using the CFG. Furthermore, I do not get warning for throw_and_catch_some() where 1.1 is a double thus catch(int &) should not catch it. The same happens in throw_catch_rethrow_the_rest(), where catch(int &) should not catch 1.1, but catch(...) should catch and rethrow it. Is this latter one a bug?

include/clang/Basic/DiagnosticSemaKinds.td

 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup<Exceptions>;
 def note_previous_exception_handler : Note<"for type %0">;
 def err_exceptions_disabled : Error<
   "cannot use '%0' with exceptions disabled">;
 def err_objc_exceptions_disabled : Error<
   "cannot use '%0' with Objective-C exceptions disabled">;
+def warn_throw_in_noexcept_func 
+    : Warning<"%0 has a non-throwing exception specification but can still "
+      "throw, resulting in unexpected program termination">,
+      InGroup<Exceptions>;
+def note_throw_in_dtor 
+    : Note<"destructor or deallocator has a (possibly implicit) non-throwing "
+      "excepton specification">;
+def note_throw_in_function 
+    : Note<"non-throwing function declare here">;
 def err_seh_try_outside_functions : Error<
   "cannot use SEH '__try' in blocks, captured regions, or Obj-C method decls">;
 def err_mixing_cxx_try_seh_try : Error<
   "cannot use C++ 'try' in the same function as SEH '__try'">;
 def err_seh_try_unsupported : Error<
   "SEH '__try' is not supported on this target">;
 def note_conflicting_try_here : Note<
   "conflicting %0 here">;

lib/Sema/AnalysisBasedWarnings.cpp

if (cfg->getExit().pred_empty())
     return;
 
   // Emit diagnostic if a recursive function call is detected for all paths.
   if (checkForRecursiveFunctionCall(FD, cfg))
     S.Diag(Body->getLocStart(), diag::warn_infinite_recursive_function);
 }
 
 //===----------------------------------------------------------------------===//
+// Check for throw in a non-throwing function.
+//===----------------------------------------------------------------------===//
+enum ThrowState {
+  FoundNoPathForThrow,
+  FoundPathForThrow,
+  FoundPathWithNoThrowOutFunction,
+};
+
+static bool isThrowCaught(const CXXThrowExpr *Throw,
+                          const CXXCatchStmt *Catch) {
+  const Type *ThrowType = nullptr;
+  if (Throw->getSubExpr())
+    ThrowType = Throw->getSubExpr()->getType().getTypePtrOrNull();
+  if (!ThrowType)
+    return false;
+  const Type *CaughtType = Catch->getCaughtType().getTypePtrOrNull();
+  if (!CaughtType)
+    return true;
+  if (ThrowType->isReferenceType())
+    ThrowType = ThrowType->castAs<ReferenceType>()
+                    ->getPointeeType()
+                    ->getUnqualifiedDesugaredType();
+  if (CaughtType->isReferenceType())
+    CaughtType = CaughtType->castAs<ReferenceType>()
+                     ->getPointeeType()
+                     ->getUnqualifiedDesugaredType();
+  if (CaughtType == ThrowType)
+    return true;
+  const CXXRecordDecl *CaughtAsRecordType =
+      CaughtType->getPointeeCXXRecordDecl();
+  const CXXRecordDecl *ThrowTypeAsRecordType = ThrowType->getAsCXXRecordDecl();
+  if (CaughtAsRecordType && ThrowTypeAsRecordType)
+    return ThrowTypeAsRecordType->isDerivedFrom(CaughtAsRecordType);
+  return false;
+}
+
+static bool isThrowCaughtByHandlers(const CXXThrowExpr *CE,
+                                    const CXXTryStmt *TryStmt) {
+  for (unsigned H = 0, E = TryStmt->getNumHandlers(); H < E; ++H) {
+    if (isThrowCaught(CE, TryStmt->getHandler(H)))
+      return true;
+  }
+  return false;
+}
+
+static bool doesThrowEscapePath(CFGBlock Block, SourceLocation &OpLoc) {
+  for (const auto &B : Block) {
+    if (B.getKind() != CFGElement::Statement)
+      continue;
+    const auto *CE = dyn_cast<CXXThrowExpr>(B.getAs<CFGStmt>()->getStmt());
+    if (!CE)
+      continue;
+
+    OpLoc = CE->getThrowLoc();
+    for (const auto &I : Block.succs()) {
+      if (!I.isReachable())
+        continue;
+      if (const auto *Terminator =
+              dyn_cast_or_null<CXXTryStmt>(I->getTerminator()))
+        if (isThrowCaughtByHandlers(CE, Terminator))
+          return false;
+    }
+    return true;
+  }
+  return false;
+}
+
+static bool hasThrowOutNonThrowingFunc(SourceLocation &OpLoc, CFG *BodyCFG) {
+
+  unsigned ExitID = BodyCFG->getExit().getBlockID();
+
+  SmallVector<ThrowState, 16> States(BodyCFG->getNumBlockIDs(),
+                                     FoundNoPathForThrow);
+  States[BodyCFG->getEntry().getBlockID()] = FoundPathWithNoThrowOutFunction;
+
+  SmallVector<CFGBlock *, 16> Stack;
+  Stack.push_back(&BodyCFG->getEntry());
+  while (!Stack.empty()) {
+    CFGBlock *CurBlock = Stack.back();
+    Stack.pop_back();
+
+    unsigned ID = CurBlock->getBlockID();
+    ThrowState CurState = States[ID];
+    if (CurState == FoundPathWithNoThrowOutFunction) {
+      if (ExitID == ID)
+        continue;
+
+      if (doesThrowEscapePath(*CurBlock, OpLoc))
+        CurState = FoundPathForThrow;
+    }
+
+    // Loop over successor blocks and add them to the Stack if their state
+    // changes.
+    for (const auto &I : CurBlock->succs())
+      if (I.isReachable()) {
+        unsigned NextID = I->getBlockID();
+        if (NextID == ExitID && CurState == FoundPathForThrow) {
+          States[NextID] = CurState;
+        } else if (States[NextID] < CurState) {
+          States[NextID] = CurState;
+          Stack.push_back(I);
+        }
+      }
+  }
+  // Return true if the exit node is reachable, and only reachable through
+  // a throw expression.
+  return States[ExitID] == FoundPathForThrow;
+}
+
+static void EmitDiagForCXXThrowInNonThrowingFunc(Sema &S, SourceLocation OpLoc,
+                                                 const FunctionDecl *FD) {
+  if (!S.getSourceManager().isInSystemHeader(OpLoc)) {
+    S.Diag(OpLoc, diag::warn_throw_in_noexcept_func) << FD;
+    if (S.getLangOpts().CPlusPlus11 &&
+        (isa<CXXDestructorDecl>(FD) ||
+         FD->getDeclName().getCXXOverloadedOperator() == OO_Delete ||
+         FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete))
+      S.Diag(FD->getLocation(), diag::note_throw_in_dtor);
+    else
+      S.Diag(FD->getLocation(), diag::note_throw_in_function);
+  }
+}
+
+static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD,
+                                        AnalysisDeclContext &AC) {
+  CFG *BodyCFG = AC.getCFG();
+  if (!BodyCFG)
+    return;
+  if (BodyCFG->getExit().pred_empty())
+    return;
+  SourceLocation OpLoc;
+  if (hasThrowOutNonThrowingFunc(OpLoc, BodyCFG))
+    EmitDiagForCXXThrowInNonThrowingFunc(S, OpLoc, FD);
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs<FunctionProtoType>();
+  if (FPT->getExceptionSpecType() != EST_None &&
+      FPT->isNothrow(FD->getASTContext()))
+    return true;
+  return false;
+}
+
+//===----------------------------------------------------------------------===//
 // Check for missing return value.
 //===----------------------------------------------------------------------===//
 
 enum ControlFlowKind {
   UnknownFallThrough,
   NeverFallThrough,
   MaybeFallThrough,
   AlwaysFallThrough,
AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
   // Check for infinite self-recursion in functions
   if (!Diags.isIgnored(diag::warn_infinite_recursive_function,
                        D->getLocStart())) {
     if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
       checkRecursiveFunction(S, FD, Body, AC);
     }
   }
 
+  // Check for throw out of non-throwing function.
+  if (!Diags.isIgnored(diag::warn_throw_in_noexcept_func, D->getLocStart()))
+    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+      if (S.getLangOpts().CPlusPlus && isNoexcept(FD))
+        checkThrowInNonThrowingFunc(S, FD, AC);
+
   // If none of the previous checks caused a CFG build, trigger one here
   // for -Wtautological-overlap-compare
   if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison,
                                D->getLocStart())) {
     AC.getCFG();
   }
 
   // Collect statistics about the CFG if it was built.

test/CXX/except/except.spec/p11.cpp

 // RUN: %clang_cc1 -std=c++11 -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 // This is the "let the user shoot themselves in the foot" clause.
-void f() noexcept {
-  throw 0; // no-error
+void f() noexcept { // expected-note {{non-throwing function declare here}}
+  throw 0; // expected-warning {{has a non-throwing exception specification but}} 
 }
-void g() throw() {
-  throw 0; // no-error
+void g() throw() { // expected-note {{non-throwing function declare here}}
+  throw 0; // expected-warning {{has a non-throwing exception specification but}} 
 }
 void h() throw(int) {
   throw 0.0; // no-error
 }

test/SemaCXX/warn-throw-out-noexcept-func.cpp

+// RUN: %clang_cc1 %s  -fdelayed-template-parsing -fcxx-exceptions -fsyntax-only -Wexceptions -verify -std=c++11
+struct A {
+  ~A();
+};         // implicitly noexcept(true)
+A::~A() {  // expected-note  {{destructor or deallocator has a}}
+  throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+struct B {
+  int i;
+  ~B() noexcept(true) {}
+};
+struct R : A {
+  B b;
+  ~R() {     // expected-note  {{destructor or deallocator has a}}
+    throw 1; // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+
+struct M : A {
+  B b;
+  ~M() noexcept(false);
+};
+
+M::~M() noexcept(false) {
+  throw 1;
+}
+
+struct N : A {
+  B b;
+  ~N(); //implicitly noexcept(true)
+};
+
+N::~N() {  // expected-note  {{destructor or deallocator has a}}
+  throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+struct X : A {
+  B b;
+  ~X() noexcept { // expected-note  {{destructor or deallocator has a}}
+    throw 1;      // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+struct Y : A {
+  B b;
+  ~Y() noexcept(true) { // expected-note  {{destructor or deallocator has a}}
+    throw 1;            // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+struct C {
+  int i;
+  ~C() noexcept(false) {}
+};
+struct D : A {
+  C c;
+  ~D() { //implicitly noexcept(false)
+    throw 1;
+  }
+};
+struct E : A {
+  C c;
+  ~E(); //implicitly noexcept(false)
+};
+E::~E() //implicitly noexcept(false)
+{
+  throw 1;
+}
+
+template <typename T>
+class A1 {
+  T b;
+
+public:
+  ~A1() {    // expected-note  {{destructor or deallocator has a}}
+    throw 1; // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+template <typename T>
+struct B1 {
+  T i;
+  ~B1() noexcept(true) {}
+};
+template <typename T>
+struct R1 : A1<T> //expected-note {{in instantiation of member function}}
+{
+  B1<T> b;
+  ~R1() {    // expected-note  {{destructor or deallocator has a}}
+    throw 1; // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+template <typename T>
+struct S1 : A1<T> {
+  B1<T> b;
+  ~S1() noexcept { // expected-note  {{destructor or deallocator has a}}
+    throw 1;       // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+void operator delete(void *ptr) noexcept { // expected-note  {{destructor or deallocator has a}}
+  throw 1;                                 // expected-warning {{has a non-throwing exception specification but}}
+}
+struct except_fun {
+  static const bool i = false;
+};
+struct noexcept_fun {
+  static const bool i = true;
+};
+template <typename T>
+struct dependent_warn {
+  ~dependent_warn() noexcept(T::i) {
+    throw 1;
+  }
+};
+template <typename T>
+struct dependent_warn_noexcept {
+  ~dependent_warn_noexcept() noexcept(T::i) { // expected-note  {{destructor or deallocator has a}}
+    throw 1;                                  // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+template <typename T>
+struct dependent_warn_both {
+  ~dependent_warn_both() noexcept(T::i) { // expected-note  {{destructor or deallocator has a}}
+    throw 1;                              // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+void foo() noexcept { //expected-note {{non-throwing function declare here}}
+  throw 1;            // expected-warning {{has a non-throwing exception specification but}}
+}
+struct Throws {
+  ~Throws()noexcept;
+};
+
+struct ShouldDiagnose {
+  Throws T;
+  ~ShouldDiagnose() { //expected-note {{destructor or deallocator has a}}
+    throw; // expected-warning {{has a non-throwing exception specification but}}
+  }
+};
+
+void bar() noexcept {
+  try {
+    throw 1;
+  } catch (...) {
+  }
+}
+void f() noexcept {
+  try {
+    throw 12;
+  } catch (int) {
+  }
+}
+void g() noexcept {
+  try {
+    throw 12;
+  } catch (...) {
+  }
+}
+
+void h() noexcept { //expected-note {{non-throwing function declare here}}
+  try {
+    throw 12; // expected-warning {{has a non-throwing exception specification but}}
+  } catch (const char *) {
+  }
+}
+
+void i() noexcept { //expected-note {{non-throwing function declare here}}
+  try {
+    throw 12;
+  } catch (int) {
+    throw; // expected-warning {{has a non-throwing exception specification but}}
+  }
+}
+void j() noexcept { //expected-note {{non-throwing function declare here}}
+  try {
+    throw 12;
+  } catch (int) {
+    throw "haha"; // expected-warning {{has a non-throwing exception specification but}}
+  }
+}
+
+void k() noexcept { //expected-note {{non-throwing function declare here}}
+  try {
+    throw 12;
+  } catch (...) {
+    throw; // expected-warning {{has a non-throwing exception specification but}}
+  }
+}
+
+void loo(int i) noexcept { //expected-note {{non-throwing function declare here}}
+  if (i)
+    try {
+      throw 12;
+    } catch (int) {
+      throw "haha"; //expected-warning {{has a non-throwing exception specification but}}
+    }
+  i = 10;
+}
+
+void loo1() noexcept {
+  if (0)
+    throw 12;
+}
+
+void loo2() noexcept { //expected-note {{non-throwing function declare here}}
+  if (1)
+    throw 12; // expected-warning {{has a non-throwing exception specification but}}
+}
+struct S {};
+
+void l() noexcept { //expected-note {{non-throwing function declare here}}
+  try {
+    throw S{}; //expected-warning {{has a non-throwing exception specification but}}
+  } catch (S *s) {
+  }
+}
+
+void m() noexcept {
+  try {
+    const S &s = S{};
+    throw s;
+  } catch (S s) {
+  }
+
+}
+void n() noexcept {
+  try {
+    S s = S{};
+    throw s;
+  } catch (const S &s) {
+  }
+}
+void o() noexcept { //expected-note {{non-throwing function declare here}}
+  try {
+    throw; //expected-warning {{has a non-throwing exception specification but}}
+  } catch (...) {
+  }
+}
+
+#define NOEXCEPT noexcept
+void with_macro() NOEXCEPT { //expected-note {{non-throwing function declare here}}
+  throw 1; // expected-warning {{has a non-throwing exception specification but}}
+}
+
+void with_try_block() try {
+  throw 2;
+} catch (...) {
+}
+
+void with_try_block1() noexcept try { //expected-note {{non-throwing function declare here}}
+  throw 2;  // expected-warning {{has a non-throwing exception specification but}}
+} catch (char *) {
+}
+
+int main() {
+  R1<int> o; //expected-note {{in instantiation of member function}}
+  S1<int> b; //expected-note {{in instantiation of member function}}
+  dependent_warn<except_fun> f;
+  dependent_warn_noexcept<noexcept_fun> f1; //expected-note {{in instantiation of member function}}
+  dependent_warn_both<except_fun> f2;
+  dependent_warn_both<noexcept_fun> f3; //expected-note {{in instantiation of member function}}
+  ShouldDiagnose obj;  
+}