aaron.ballman requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:25
@@ +24,3 @@
+ Finder->addMatcher(
+ cxxThrowExpr(stmt(hasAncestor(functionDecl(isNoThrow()).bind("func"))))
+ .bind("throw"),
----------------
Prazek wrote:
> you can use forFunction instead of hasAncestor(functionDecl(
> cxxThrowExpr(stmt(forFunction(isNoThrow() or
> cxxThrowExpr(stmt(forFunction(functionDecl(isNoThrow())))
> you can even try to remove stmt.
>
>
> Also add test case
>
> void getFunction() noexcept {
> struct X {
> static void inner()
> {
> throw 42;
> }
> };
> }
>
Hmm, I wonder if it's trivial enough to filter out throw statements that are
inside a try block scope (even without checking the expression and catch block
types)?
================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:32
@@ +31,3 @@
+// token including trailing whitespace.
+static SourceRange FindToken(const SourceManager &Sources, LangOptions
LangOpts,
+ SourceLocation StartLoc, SourceLocation EndLoc,
----------------
Prazek wrote:
> function in llvm starts with lower case
I prefer this be gated on D20428; we should be pushing this functionality down
where it belongs instead of replicating it in several different clang-tidy
checks.
================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:91
@@ +90,3 @@
+
+SourceRange FindNoExceptRange(const ASTContext &Context,
+ const SourceManager &Sources,
----------------
Prazek wrote:
> rename it to findNoexceptRange at mark it static.
This function will not work with dynamic exception specifications that also
specify the function is nonthrowing, e.g., `throw()`.
================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:130
@@ +129,3 @@
+ const auto *Throw = Result.Nodes.getNodeAs<Stmt>("throw");
+ diag(Throw->getLocStart(), "throw in function declared no-throw");
+ for (auto Redecl : Function->redecls()) {
----------------
Prazek wrote:
> add new line after this line
How about wording the diagnostic: "'throw' expression in a function declared
with a non-throwing exception specification"
================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:131
@@ +130,3 @@
+ diag(Throw->getLocStart(), "throw in function declared no-throw");
+ for (auto Redecl : Function->redecls()) {
+ SourceRange NoExceptRange =
----------------
Prazek wrote:
> add some new line
`const auto *`, please (assuming Redecl is a pointer type).
================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20
@@ +19,3 @@
+///\brief Warns about using throw in function declared as noexcept.
+/// It complains about every throw, even if it is caught later.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {
----------------
What is the false positive rate with this check over a large codebase that uses
exceptions?
================
Comment at: test/clang-tidy/misc-throw-with-noexcept.cpp:70
@@ +69,2 @@
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: throw in function declared
no-throw [misc-throw-with-noexcept]
----------------
There are no tests for dynamic exception specifications.
Repository:
rL LLVM
http://reviews.llvm.org/D19201
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
