[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336997: [clang-tidy] Exception Escape Checker (authored by 
baloghadamsoftware, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D33537?vs=151876=155366#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33537

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-exception-escape.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/test/clang-tidy/bugprone-exception-escape.cpp

Index: clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -0,0 +1,214 @@
+//===--- ExceptionEscapeCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ExceptionEscapeCheck.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringSet.h"
+
+using namespace clang::ast_matchers;
+
+namespace {
+typedef llvm::SmallVector TypeVec;
+} // namespace
+
+namespace clang {
+
+static bool isBaseOf(const Type *DerivedType, const Type *BaseType) {
+  const auto *DerivedClass = DerivedType->getAsCXXRecordDecl();
+  const auto *BaseClass = BaseType->getAsCXXRecordDecl();
+  if (!DerivedClass || !BaseClass)
+return false;
+
+  return !DerivedClass->forallBases(
+  [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; });
+}
+
+static const TypeVec
+throwsException(const Stmt *St, const TypeVec ,
+llvm::SmallSet );
+
+static const TypeVec
+throwsException(const FunctionDecl *Func,
+llvm::SmallSet ) {
+  if (CallStack.count(Func))
+return TypeVec();
+
+  if (const Stmt *Body = Func->getBody()) {
+CallStack.insert(Func);
+const TypeVec Result = throwsException(Body, TypeVec(), CallStack);
+CallStack.erase(Func);
+return Result;
+  }
+
+  TypeVec Result;
+  if (const auto *FPT = Func->getType()->getAs()) {
+for (const QualType Ex : FPT->exceptions()) {
+  Result.push_back(Ex.getTypePtr());
+}
+  }
+  return Result;
+}
+
+static const TypeVec
+throwsException(const Stmt *St, const TypeVec ,
+llvm::SmallSet ) {
+  TypeVec Results;
+
+  if (!St)
+return Results;
+
+  if (const auto *Throw = dyn_cast(St)) {
+if (const auto *ThrownExpr = Throw->getSubExpr()) {
+  const auto *ThrownType =
+  ThrownExpr->getType()->getUnqualifiedDesugaredType();
+  if (ThrownType->isReferenceType()) {
+ThrownType = ThrownType->castAs()
+ ->getPointeeType()
+ ->getUnqualifiedDesugaredType();
+  }
+  if (const auto *TD = ThrownType->getAsTagDecl()) {
+if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc"
+&& TD->isInStdNamespace())
+  return Results;
+  }
+  Results.push_back(ThrownExpr->getType()->getUnqualifiedDesugaredType());
+} else {
+  Results.append(Caught.begin(), Caught.end());
+}
+  } else if (const auto *Try = dyn_cast(St)) {
+TypeVec Uncaught = throwsException(Try->getTryBlock(), Caught, CallStack);
+for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {
+  const CXXCatchStmt *Catch = Try->getHandler(i);
+  if (!Catch->getExceptionDecl()) {
+const TypeVec Rethrown =
+throwsException(Catch->getHandlerBlock(), Uncaught, CallStack);
+Results.append(Rethrown.begin(), Rethrown.end());
+Uncaught.clear();
+  } else {
+const auto *CaughtType =
+Catch->getCaughtType()->getUnqualifiedDesugaredType();
+if (CaughtType->isReferenceType()) {
+  CaughtType = CaughtType->castAs()
+   ->getPointeeType()
+   ->getUnqualifiedDesugaredType();
+}
+auto NewEnd =
+llvm::remove_if(Uncaught, [](const Type *ThrownType) {
+  return ThrownType == CaughtType ||
+ isBaseOf(ThrownType, CaughtType);
+});
+if (NewEnd != Uncaught.end()) {
+  Uncaught.erase(NewEnd, Uncaught.end());
+  const TypeVec Rethrown = 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good with one comment.




Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
+  implicit_int_thrower();

baloghadamsoftware wrote:
> dberris wrote:
> > Can we make the warning more accurate here? Something like:
> > 
> > ```
> > warning: call to 'implicit_int_thrower' may throw an exception and 
> > propagate through noexcept function 'indirect_implicit'
> > ```
> > 
> > It would be helpful to diagnose the point at which the exception may be 
> > thrown from within the function (if it's an operator, a function call, 
> > etc.) that doesn't have exceptions handled. If you can highlight not just 
> > the line number but the actual expression in which the uncaught exception 
> > may propagate, it would make this warning much better.
> > 
> > If you think it's worth it (or if it's feasible), having a FixIt hint to 
> > wrap a block of statements where exceptions may propagate in a `try { ... } 
> > catch (...) { ... }` block would turn this warning from a good warning, to 
> > a great warning -- potentially something that could be automatically 
> > applied by a tool as well.
> I think this is a good future improvement. However, I tried to make the first 
> version as small as possible, and then enhance it incrementally. Even the 
> current code is too big for a Clang-Tidy check.
Could you add a relevant FIXME comment next to the diag() call in the check?


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Any news regarding this?


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 151876.
baloghadamsoftware added a comment.

Typo fixed.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,265 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.FunctionsThatShouldNotThrow, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function '~throwing_destructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function 'throwing_move_constructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: an exception may be thrown in function 'operator=' which should not throw exceptions
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_noexcept' which should not throw exceptions
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_throw_nothing' which should not throw exceptions
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_some' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_each' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_all' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_rethrow' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_throw' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_rethrow_the_rest' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base' which should not throw exceptions
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'try_nested_try' which should not throw exceptions
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'bad_try_nested_try' which should not throw exceptions
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
+  implicit_int_thrower();

dberris wrote:
> Can we make the warning more accurate here? Something like:
> 
> ```
> warning: call to 'implicit_int_thrower' may throw an exception and propagate 
> through noexcept function 'indirect_implicit'
> ```
> 
> It would be helpful to diagnose the point at which the exception may be 
> thrown from within the function (if it's an operator, a function call, etc.) 
> that doesn't have exceptions handled. If you can highlight not just the line 
> number but the actual expression in which the uncaught exception may 
> propagate, it would make this warning much better.
> 
> If you think it's worth it (or if it's feasible), having a FixIt hint to wrap 
> a block of statements where exceptions may propagate in a `try { ... } catch 
> (...) { ... }` block would turn this warning from a good warning, to a great 
> warning -- potentially something that could be automatically applied by a 
> tool as well.
I think this is a good future improvement. However, I tried to make the first 
version as small as possible, and then enhance it incrementally. Even the 
current code is too big for a Clang-Tidy check.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

I'm really excited by this clang-tidy check and I think it's worth having. I do 
think there's more work needed to bring it up to a level of quality that would 
be helpful to more users.




Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:6
+
+Finds functions which may throw an excpetion directly or indirectly, but they
+should not. The functions which should not throw exceptions are the following:

excpetion -> exception



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
+  implicit_int_thrower();

Can we make the warning more accurate here? Something like:

```
warning: call to 'implicit_int_thrower' may throw an exception and propagate 
through noexcept function 'indirect_implicit'
```

It would be helpful to diagnose the point at which the exception may be thrown 
from within the function (if it's an operator, a function call, etc.) that 
doesn't have exceptions handled. If you can highlight not just the line number 
but the actual expression in which the uncaught exception may propagate, it 
would make this warning much better.

If you think it's worth it (or if it's feasible), having a FixIt hint to wrap a 
block of statements where exceptions may propagate in a `try { ... } catch 
(...) { ... }` block would turn this warning from a good warning, to a great 
warning -- potentially something that could be automatically applied by a tool 
as well.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 151666.
baloghadamsoftware added a comment.

New warning message, more detailed docs.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,265 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.FunctionsThatShouldNotThrow, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function '~throwing_destructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function 'throwing_move_constructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: an exception may be thrown in function 'operator=' which should not throw exceptions
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_noexcept' which should not throw exceptions
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_throw_nothing' which should not throw exceptions
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_some' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_each' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_all' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_rethrow' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_throw' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_rethrow_the_rest' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base' which should not throw exceptions
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'try_nested_try' which should not throw exceptions
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'bad_try_nested_try' which should not throw exceptions
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:6
+
+Finds functions which should not throw exceptions:
+* Destructors

I don't think the check just finds functions that should not throw exceptions. 
I suppose, it finds `throw` statements inside them (or maybe it goes slightly 
deeper - the documentation should explain this in more detail). See also the 
comment @dberris left below.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:31
+   ``WinMain()`` in the Windows API to the list of the funcions which should
+   not throw.. Default value is empty string.
+

nit: Double period after `throw`.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:31
+   ``WinMain()`` in the Windows API to the list of the funcions which should
+   not throw.. Default value is empty string.
+

alexfh wrote:
> nit: Double period after `throw`.
nit: "an empty string"
Same below.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

baloghadamsoftware wrote:
> lebedev.ri wrote:
> > baloghadamsoftware wrote:
> > > dberris wrote:
> > > > How deep does this go? Say we have a call to a function that's extern 
> > > > which doesn't have 'noexcept' nor a dynamic exception specifier -- do 
> > > > we assume that the call to an extern function may throw? Does that 
> > > > warn? What does the warning look like? Should it warn? How about when 
> > > > you call a function through a function pointer?
> > > > 
> > > > The documentation should cover these cases and/or more explicitly say 
> > > > in the warning that an exception may throw in a noexcept function 
> > > > (rather than just "function <...> throws").
> > > We take the most conservative way here. We assume that a function throws 
> > > if and only if its body has a throw statement or its header has the (old) 
> > > throw() exception specification. We do not follow function pointers.
> > While i realize it may have more noise, it may be nice to have a more 
> > pedantic mode (guarded by an option?).
> > E.g. `noexcept` propagation, much like `const` on methods propagation.
> > Or at least a test that shows that it does not happen, unless i simply did 
> > not notice it :)
> This could be an enhancement in the future, yes.
Please address the original comment here. In particular, the warning message 
should be clearer (that an exception may be thrown in a function that should 
not throw exceptions).


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

lebedev.ri wrote:
> baloghadamsoftware wrote:
> > dberris wrote:
> > > How deep does this go? Say we have a call to a function that's extern 
> > > which doesn't have 'noexcept' nor a dynamic exception specifier -- do we 
> > > assume that the call to an extern function may throw? Does that warn? 
> > > What does the warning look like? Should it warn? How about when you call 
> > > a function through a function pointer?
> > > 
> > > The documentation should cover these cases and/or more explicitly say in 
> > > the warning that an exception may throw in a noexcept function (rather 
> > > than just "function <...> throws").
> > We take the most conservative way here. We assume that a function throws if 
> > and only if its body has a throw statement or its header has the (old) 
> > throw() exception specification. We do not follow function pointers.
> While i realize it may have more noise, it may be nice to have a more 
> pedantic mode (guarded by an option?).
> E.g. `noexcept` propagation, much like `const` on methods propagation.
> Or at least a test that shows that it does not happen, unless i simply did 
> not notice it :)
This could be an enhancement in the future, yes.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

baloghadamsoftware wrote:
> dberris wrote:
> > How deep does this go? Say we have a call to a function that's extern which 
> > doesn't have 'noexcept' nor a dynamic exception specifier -- do we assume 
> > that the call to an extern function may throw? Does that warn? What does 
> > the warning look like? Should it warn? How about when you call a function 
> > through a function pointer?
> > 
> > The documentation should cover these cases and/or more explicitly say in 
> > the warning that an exception may throw in a noexcept function (rather than 
> > just "function <...> throws").
> We take the most conservative way here. We assume that a function throws if 
> and only if its body has a throw statement or its header has the (old) 
> throw() exception specification. We do not follow function pointers.
While i realize it may have more noise, it may be nice to have a more pedantic 
mode (guarded by an option?).
E.g. `noexcept` propagation, much like `const` on methods propagation.
Or at least a test that shows that it does not happen, unless i simply did not 
notice it :)


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 10 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

dberris wrote:
> How deep does this go? Say we have a call to a function that's extern which 
> doesn't have 'noexcept' nor a dynamic exception specifier -- do we assume 
> that the call to an extern function may throw? Does that warn? What does the 
> warning look like? Should it warn? How about when you call a function through 
> a function pointer?
> 
> The documentation should cover these cases and/or more explicitly say in the 
> warning that an exception may throw in a noexcept function (rather than just 
> "function <...> throws").
We take the most conservative way here. We assume that a function throws if and 
only if its body has a throw statement or its header has the (old) throw() 
exception specification. We do not follow function pointers.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145452.
baloghadamsoftware added a comment.

Updated according to the comments.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,265 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.FunctionsThatShouldNotThrow, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1;
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:154-155
+  if (const auto *TD = ThrownType->getAsTagDecl()) {
+if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc")
+  return Results;
+  }

Does this actually catch `std::bad_alloc` or just any exception called 
`bad_alloc`? Should this be a fully qualified type, those defined by the 
implementation? I suspect this isn't as simple to figure out, because an 
implementation may be using nested namespaces (inline namespace?) to bring in 
the `bad_alloc` type in the `std` namespace.



Comment at: docs/ReleaseNotes.rst:230-233
+<<< HEAD
+===
 - The 'google-runtime-member-string-references' check was removed.
+>>> master

You probably want to fix this...



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

baloghadamsoftware wrote:
> dberris wrote:
> > `EnabledFunctions` but they *should not* throw?
> Maybe we should come up with a better name for this option. I just took this 
> from our company internal check.
My suggestion here would be something like 'FunctionBlacklist' or 
'IgnoreFunctions'.

Are these exact names, or regular expressions? Should they be regular 
expressions? How about those that are in namespaces?

You might want to explore being able to configure this similar to the Sanitizer 
blacklist, which supports function name globs. I can imagine this immediately 
getting really brittle too.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

How deep does this go? Say we have a call to a function that's extern which 
doesn't have 'noexcept' nor a dynamic exception specifier -- do we assume that 
the call to an extern function may throw? Does that warn? What does the warning 
look like? Should it warn? How about when you call a function through a 
function pointer?

The documentation should cover these cases and/or more explicitly say in the 
warning that an exception may throw in a noexcept function (rather than just 
"function <...> throws").


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Thank you for the updates. A few more comments.




Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:150
+  } else if (const auto *Try = dyn_cast(St)) {
+auto Uncaught = throwsException(Try->getTryBlock(), Caught);
+for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {

alexfh wrote:
> Please use the concrete type here and below. 
> http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
>  says 
> 
> > Don’t “almost always” use auto, but do use auto with initializers like 
> > cast(...) or other places where the type is already obvious from the 
> > context.
I don't remember where this comment was, but now I see a few more instances of 
the same issue. Specifically, ThrownExpr and ThrownType here and CaughtType 
below.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:17
+
+using namespace clang;
+using namespace clang::ast_matchers;

You can open namespace clang here instead of this using directive.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:22
+typedef llvm::SmallVector TypeVec;
+typedef llvm::SmallSet StringSet;
+} // namespace

There's `llvm::StringSet<>` in llvm/ADT/StringSet.h.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:25
+
+static const TypeVec throwsException(const FunctionDecl *Func);
+

Too many forward declarations for my taste. Can you just move all static 
functions here and remove unnecessary forward declarations? I guess, one should 
be enough for all the functions here.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:178
+}
+auto *NewEnd =
+llvm::remove_if(Uncaught, [](const Type *ThrownType) {

iterator being a pointer is an implementation detail of SmallVector. s/auto 
\*/auto /



Comment at: docs/ReleaseNotes.rst:230
 
+<<< HEAD
+===

Resolve the conflicts.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

baloghadamsoftware wrote:
> dberris wrote:
> > `EnabledFunctions` but they *should not* throw?
> Maybe we should come up with a better name for this option. I just took this 
> from our company internal check.
FunctionsThatShouldNotThrow?



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:29
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the
+   Windows API. Default vale is empty string.

Examples should be copy-pastable. I suppose, one can't use `WinMain()` verbatim 
as the value of this option. An example could be: `main,WinMain` or something 
like that.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:30
+   example for using this parameter is the function ``WinMain()`` in the
+   Windows API. Default vale is empty string.
+

s/vale/value/


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145174.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

New test added.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,263 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1;
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:8-9
+* Destructors
+* Copy constructors
+* Copy assignment operators
+* The ``main()`` functions

dberris wrote:
> Are copy constructors and assignment operators not supposed to throw? Or did 
> you mean *move* constructors?
You are right. In the description below it is correct, but here there was a 
typo.



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

dberris wrote:
> `EnabledFunctions` but they *should not* throw?
Maybe we should come up with a better name for this option. I just took this 
from our company internal check.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145172.
baloghadamsoftware added a comment.

Typo fixed.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1;
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

How about tests for functions with conditional noexcept?




Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:8-9
+* Destructors
+* Copy constructors
+* Copy assignment operators
+* The ``main()`` functions

Are copy constructors and assignment operators not supposed to throw? Or did 
you mean *move* constructors?



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

`EnabledFunctions` but they *should not* throw?


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#1086571, @alexfh wrote:

> It looks like you've missed some comments or uploaded a wrong patch.


The latter. Now I retried to upload the correct patch.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 145155.
baloghadamsoftware added a comment.

Retrying...


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

It looks like you've missed some comments or uploaded a wrong patch.




Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:105
+const TypeVec throwsException(const FunctionDecl *Func) {
+  static thread_local llvm::SmallSet CallStack;
+

alexfh wrote:
> I don't think this is a good use case for a static local variable. If this 
> function needs a state, either pass it from the caller or create a utility 
> class/struct for it. You can leave a non-recursive entry point with the 
> current interface, if you like. For example:
> 
>   const TypeVec throwsException(const FunctionDecl *Func, 
> llvm::SmallSet *CallStack) { ... }
> 
>   const TypeVec throwsException(const FunctionDecl *Func) {
> llvm::SmallSet CallStack;
> return throwsException(Func, );
>   }
>   
Still not addressed?



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:110
+
+  if (const auto *Body = Func->getBody()) {
+CallStack.insert(Func);

alexfh wrote:
> Please use the concrete type here, since it's not obvious from the context.
Still not addressed?



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:119
+  if (const auto *FPT = Func->getType()->getAs()) {
+for (const auto Ex : FPT->exceptions()) {
+  Result.push_back(&*Ex);

alexfh wrote:
> Please use the concrete type here.
Still not addressed?



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:120
+for (const auto Ex : FPT->exceptions()) {
+  Result.push_back(&*Ex);
+}

alexfh wrote:
> Ex.getTypePtrOrNull() / Ex.getTypePtr() would be easier to understand here.
Still not addressed?


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 143068.
baloghadamsoftware added a comment.

Updated according to the comments.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Thank you for your thorough review, I will do the fixes next week, after the 
Euro LLVM.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:100
+
+namespace {
+

> make anonymous namespaces as small as possible, and only use them for class 
> declarations

http://llvm.org/docs/CodingStandards.html#anonymous-namespaces



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:105
+const TypeVec throwsException(const FunctionDecl *Func) {
+  static thread_local llvm::SmallSet CallStack;
+

I don't think this is a good use case for a static local variable. If this 
function needs a state, either pass it from the caller or create a utility 
class/struct for it. You can leave a non-recursive entry point with the current 
interface, if you like. For example:

  const TypeVec throwsException(const FunctionDecl *Func, llvm::SmallSet *CallStack) { ... }

  const TypeVec throwsException(const FunctionDecl *Func) {
llvm::SmallSet CallStack;
return throwsException(Func, );
  }
  



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:110
+
+  if (const auto *Body = Func->getBody()) {
+CallStack.insert(Func);

Please use the concrete type here, since it's not obvious from the context.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:119
+  if (const auto *FPT = Func->getType()->getAs()) {
+for (const auto Ex : FPT->exceptions()) {
+  Result.push_back(&*Ex);

Please use the concrete type here.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:120
+for (const auto Ex : FPT->exceptions()) {
+  Result.push_back(&*Ex);
+}

Ex.getTypePtrOrNull() / Ex.getTypePtr() would be easier to understand here.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:150
+  } else if (const auto *Try = dyn_cast(St)) {
+auto Uncaught = throwsException(Try->getTryBlock(), Caught);
+for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {

Please use the concrete type here and below. 
http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
 says 

> Don’t “almost always” use auto, but do use auto with initializers like 
> cast(...) or other places where the type is already obvious from the 
> context.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:166
+}
+auto *NewEnd = std::remove_if(Uncaught.begin(), Uncaught.end(),
+  [](const Type *ThrownType) {

1. iterator being a pointer is an implementation detail of SmallVector
2. use llvm::remove_if, it has a range based interface



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:176
+  Results.append(Rethrown.begin(), Rethrown.end());
+  Uncaught.erase(NewEnd, Uncaught.end());
+}

I'd put this as close to the remove_if as possible to allow readers keep less 
context to understand the code. In this case - as the first statement inside 
the `if`.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 140117.
baloghadamsoftware added a comment.

,html removed from release notes.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:68
+- New :doc:`bugprone-exception-escape
+  ` check
+

.html is not necessary.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 4 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:24-25
+
+const TypeVec _throws(const FunctionDecl *Func);
+const TypeVec _throws(const Stmt *St, const TypeVec );
+} // namespace

alexfh wrote:
> alexfh wrote:
> > Function names should follow llvm conventions 
> > http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> If there's no need to pass nullptr to these functions, the arguments can be 
> const references.
I agree in general, but I usually get `Stmt*` and `FunctionDecl*` from query 
functions, so it is simpler to pass them as they are instead of dereferencing 
them. The too many asterisks decrease the readability of the code. Furthermore, 
St may be `nullptr` in the second function.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 140079.
baloghadamsoftware added a comment.

Updated according to the comments.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:24-25
+
+const TypeVec _throws(const FunctionDecl *Func);
+const TypeVec _throws(const Stmt *St, const TypeVec );
+} // namespace

Function names should follow llvm conventions 
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:24-25
+
+const TypeVec _throws(const FunctionDecl *Func);
+const TypeVec _throws(const Stmt *St, const TypeVec );
+} // namespace

alexfh wrote:
> Function names should follow llvm conventions 
> http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
If there's no need to pass nullptr to these functions, the arguments can be 
const references.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:115
+return Result;
+  } else {
+TypeVec Result;

No `else` after `return`.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:142
+  if (const auto *TD = ThrownType->getAsTagDecl()) {
+if (TD->getNameAsString() == "bad_alloc")
+  return Results;

Don't use getNameAsString when not necessary. `if 
(TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc")` will work 
as good, but without unnecessary string copies.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:68
+- New :doc:`bugprone-exception-escape
+  
` 
check
+

Please fix link. See other checks as example.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: docs/clang-tidy/checks/misc-exception-escape.rst:1
+.. title:: clang-tidy - bugprone-exception-escape
+

This file should be renamed as well.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 139583.
baloghadamsoftware added a comment.

Moved to bugprone.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Escaping exception is certainly a bug, so it should go into `bugprone` then.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: rnkovacs.

Is there a more specific module for this check than misc? For example, does it 
check a rule that happens to appear in a certain coding standard? Otherwise, 
maybe "bugprone-"? I've moved a number of checks out of misc- and it would be 
nice to avoid putting more stuff there.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

We take the conservative approach there: if a function has no `throw` specifier 
we handle it as it would have a `noexcept` specifier.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[JVApen via Phabricator via cfe-commits]

JVApen added a comment.

Looking at the documentation and the examples, it is unclear to me what the 
behavior of checking is when you call a  function without throw/noexcept if you 
can't see the implementation. (Other cpp file)


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#902242, @JonasToth wrote:

>




> Will this check find stuff like this (or is it part of the frontend)
> 
>   int throwing() {
>   throw int(42);
>   }
>   
>   int not_throwing() noexcept {
>   throwing();
>   }

Yes, see lines 171-193 of the test file.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> I agree that we should not spend too much effort on making warnings from the 
> compiler and tidy disjunct.

+1
There is an effort to treat clangs frontend warnings similar to clang-tidy 
checks within clang-tidy. This would allow to manage the overlap as well.

Will this check find stuff like this (or is it part of the frontend)

  int throwing() {
  throw int(42);
  }
  
  int not_throwing() noexcept {
  throwing();
  }

It would be nice to have a check that could automatically introduce `noexcept` 
into a codebase for cases it safe. I think this path would be a good place for 
it.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I agree that we should not spend too much effort on making warnings from the 
compiler and tidy disjunct.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I think that the acceptable false positive rate of a compiler warning is much 
lower than that of a Tidy check. So I understand that the fronted patch will 
not handle the indirect cases (which are the most important in my opinion) or 
the cases where there are multiple throws in a try block. However, it is nearly 
impossible to remove the most straightforward cases from the check and also in 
today's Tidy checks or Static Analyzer checkers there is some overlapping with 
warnings. I think that maybe we should consider making the error locations and 
error text to overlap with the warning so an advanced tool (such as 
CodeChecker) could unify them.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

In https://reviews.llvm.org/D33537#827509, @baloghadamsoftware wrote:

> Test changed. I made some bad throws reachable, but the frontend check still 
> does detects them.


If block contains two or more throws, that mean compiler can not statically 
know throw is really throws.  So to avoid fails positive, if one of throw can 
be catch, compiler don't emit warning.

Jennifer


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 109126.
baloghadamsoftware added a comment.
Herald added a subscriber: JDevlieghere.

Test changed. I made some bad throws reachable, but the frontend check still 
does detects them.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:

> In https://reviews.llvm.org/D33537#771159, @baloghadamsoftware wrote:
>
> > In https://reviews.llvm.org/D33537#770264, @aaron.ballman wrote:
> >
> > > I think we should try to get as much of this functionality in 
> > > https://reviews.llvm.org/D3 as possible; there is a considerable 
> > > amount of overlap between that functionality and this functionality. This 
> > > check can then cover only the parts that are not reasonably handled by 
> > > the frontend check instead of duplicating diagnostics the user already 
> > > receives.
> >
> >
> > I suppose that the frontend check will not travarse the call-graph just 
> > check direct throws. Should we only check indirect throws then?
>
>
> The check in https://reviews.llvm.org/D3 is using a CFG, not just 
> checking direct throws.




In https://reviews.llvm.org/D33537#807997, @erichkeane wrote:

> In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote:
>
> > In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:
> >
> > > The check in https://reviews.llvm.org/D3 is using a CFG, not just 
> > > checking direct throws.
> >
> >
> > I tested the latest revision (the fronted patch already included) on my 
> > test file. Disregarding of the not so important parameters 
> > (`EnabledFunctions` and `IgnoredExceptions`) I do not get warnings for any 
> > indirect throws (`indirect_implicit()` and `indirect_explicit()`). So for 
> > me it 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. This latter may be a bug.
>
>
> Jen Yu can clarify, but I believe it was decided to put the implementation in 
> the CFG, but not do a full analysis (leave that for a later implementation).  
> It is not doing 'catch' analysis, and I don't believe they decided to do 
> analysis on the function call itself, since the false-positive rate is 
> massive thanks to the C headers.


The catch  analys

In https://reviews.llvm.org/D33537#807997, @erichkeane wrote:

> In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote:
>
> > In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:
> >
> > > The check in https://reviews.llvm.org/D3 is using a CFG, not just 
> > > checking direct throws.
> >
> >
> > I tested the latest revision (the fronted patch already included) on my 
> > test file. Disregarding of the not so important parameters 
> > (`EnabledFunctions` and `IgnoredExceptions`) I do not get warnings for any 
> > indirect throws (`indirect_implicit()` and `indirect_explicit()`). So for 
> > me it 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. This latter may be a bug.
>
>
> Jen Yu can clarify, but I believe it was decided to put the implementation in 
> the CFG, but not do a full analysis (leave that for a later implementation).  
> It is not doing 'catch' analysis, and I don't believe they decided to do 
> analysis on the function call itself, since the false-positive rate is 
> massive thanks to the C headers.




In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:
>
> > The check in https://reviews.llvm.org/D3 is using a CFG, not just 
> > checking direct throws.
>
>
> I tested the latest revision (the fronted patch already included) on my test 
> file. Disregarding of the not so important parameters (`EnabledFunctions` and 
> `IgnoredExceptions`) I do not get warnings for any indirect throws 
> (`indirect_implicit()` and `indirect_explicit()`). So for me it 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. This latter may be a bug.




In https://reviews.llvm.org/D33537#807997, @erichkeane wrote:

> In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote:
>
> > In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:
> >
> > > The check in https://reviews.llvm.org/D3 is using a CFG, not just 
> > > checking direct throws.
> >
> >
> > I tested the latest revision (the fronted patch already included) on my 
> > test file. Disregarding of the not so important parameters 
> > (`EnabledFunctions` and 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D33537#801889, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:
>
> > The check in https://reviews.llvm.org/D3 is using a CFG, not just 
> > checking direct throws.
>
>
> I tested the latest revision (the fronted patch already included) on my test 
> file. Disregarding of the not so important parameters (`EnabledFunctions` and 
> `IgnoredExceptions`) I do not get warnings for any indirect throws 
> (`indirect_implicit()` and `indirect_explicit()`). So for me it 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. This latter may be a bug.


Jen Yu can clarify, but I believe it was decided to put the implementation in 
the CFG, but not do a full analysis (leave that for a later implementation).  
It is not doing 'catch' analysis, and I don't believe they decided to do 
analysis on the function call itself, since the false-positive rate is massive 
thanks to the C headers.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#771274, @aaron.ballman wrote:

> The check in https://reviews.llvm.org/D3 is using a CFG, not just 
> checking direct throws.


I tested the latest revision (the fronted patch already included) on my test 
file. Disregarding of the not so important parameters (`EnabledFunctions` and 
`IgnoredExceptions`) I do not get warnings for any indirect throws 
(`indirect_implicit()` and `indirect_explicit()`). So for me it 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. This latter may be a bug.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D33537#771159, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D33537#770264, @aaron.ballman wrote:
>
> > I think we should try to get as much of this functionality in 
> > https://reviews.llvm.org/D3 as possible; there is a considerable amount 
> > of overlap between that functionality and this functionality. This check 
> > can then cover only the parts that are not reasonably handled by the 
> > frontend check instead of duplicating diagnostics the user already receives.
>
>
> I suppose that the frontend check will not travarse the call-graph just check 
> direct throws. Should we only check indirect throws then?


The check in https://reviews.llvm.org/D3 is using a CFG, not just checking 
direct throws.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#770264, @aaron.ballman wrote:

> I think we should try to get as much of this functionality in 
> https://reviews.llvm.org/D3 as possible; there is a considerable amount 
> of overlap between that functionality and this functionality. This check can 
> then cover only the parts that are not reasonably handled by the frontend 
> check instead of duplicating diagnostics the user already receives.


I suppose that the frontend check will not travarse the call-graph just check 
direct throws. Should we only check indirect throws then?


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D33537#765445, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D33537#764834, @Prazek wrote:
>
> > How is that compared to https://reviews.llvm.org/D19201 and the clang patch 
> > mentioned in this patch?
>
>
> Actually, this check does much more. It does not only check for noexcept (and 
> throw()) functions, but also for destructors, move constructors, move 
> assignments, the main() function, swap() functions and also functions given 
> as option. A more important difference is that we traverse the whole 
> call-chain and check all the throw statements and try-catch blocks so 
> indirectly throwing functions are also reported and no flase positives are 
> caused by throw and catch in the same try block.


I think we should try to get as much of this functionality in 
https://reviews.llvm.org/D3 as possible; there is a considerable amount of 
overlap between that functionality and this functionality. This check can then 
cover only the parts that are not reasonably handled by the frontend check 
instead of duplicating diagnostics the user already receives.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100382.
baloghadamsoftware added a comment.

Ignoring bad_alloc.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_thrower();
+}
+
+void indirect_explicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100374.
baloghadamsoftware added a comment.

Check added to the Release Notes.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_thrower();
+}
+
+void indirect_explicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#764834, @Prazek wrote:

> How is that compared to https://reviews.llvm.org/D19201 and the clang patch 
> mentioned in this patch?


Actually, this check does much more. It does not only check for noexcept (and 
throw()) functions, but also for destructors, move constructors, move 
assignments, the main() function, swap() functions and also functions given as 
option. A more important difference is that we traverse the whole call-chain 
and check all the throw statements and try-catch blocks so indirectly throwing 
functions are also reported and no flase positives are caused by throw and 
catch in the same try block.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100369.
baloghadamsoftware added a comment.

Docs fixed according to the comments.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_thrower();
+}
+
+void indirect_explicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added a comment.

How is that compared to https://reviews.llvm.org/D19201 and the clang patch 
mentioned in this patch?


Repository:
  rL LLVM

https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).




Comment at: docs/clang-tidy/checks/misc-exception-escape.rst:7
+Finds functions which should not throw exceptions:
++ Destructors
++ Copy constructors

I think - or * is more correct.



Comment at: docs/clang-tidy/checks/misc-exception-escape.rst:10
++ Copy assignment operators
++ The main() functions
++ swap() functions

Please enclose main() in ``. Same for other functions and keywords.



Comment at: docs/clang-tidy/checks/misc-exception-escape.rst:29
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function WinMain in the Windows API.
+   Default vale is empty string.

Please add () to WinMain and enclose it in ``.



Comment at: docs/clang-tidy/checks/misc-exception-escape.rst:35
+   Comma separated list containing type names which are not counted as thrown
+   exceptions in the checker. Default value is empty string.

Please use check instead of checker.


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Supersedes https://reviews.llvm.org/D32350


https://reviews.llvm.org/D33537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
Herald added a subscriber: mgorny.

Finds functions which should not throw exceptions: Destructors, move 
constructors, move assignment operators, the main() function, swap() functions, 
functions marked with throw() or noexcept and functions given as option to the 
checker.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: