sbarzowski updated this revision to Diff 59277.
sbarzowski added a comment.
Thanks for review, guys. I fixed as many easy problems as I had time for today,
will continue later.
Repository:
rL LLVM
http://reviews.llvm.org/D19201
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/ThrowWithNoexceptCheck.cpp
clang-tidy/misc/ThrowWithNoexceptCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-throw-with-noexcept.rst
test/clang-tidy/misc-throw-with-noexcept.cpp
Index: test/clang-tidy/misc-throw-with-noexcept.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-throw-with-noexcept.cpp
@@ -0,0 +1,85 @@
+// RUN: %check_clang_tidy %s misc-throw-with-noexcept %t
+
+void f_throw_with_ne() noexcept(true) {
+ {
+ throw 5;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:24: warning: In function declared no-throw here: [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-FIXES: void f_throw_with_ne() {
+
+void f_noexcept_false() noexcept(false) {
+ throw 5;
+}
+
+void f_decl_only() noexcept;
+
+// Controversial example
+void throw_and_catch() noexcept(true) {
+ try {
+ throw 5;
+ } catch (...) {
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:24: warning: In function declared no-throw here: [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-5]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-FIXES: void throw_and_catch() {
+
+void with_parens() noexcept((true)) {
+ throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:20: warning: In function declared no-throw here: [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-3]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-FIXES: void with_parens() {
+
+
+void with_parens_and_spaces() noexcept ( (true) ) {
+ throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:31: warning: In function declared no-throw here: [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-3]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-FIXES: void with_parens_and_spaces() {
+
+class Class {
+ void InClass() const noexcept(true) {
+ throw 42;
+ }
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:24: warning: In function declared no-throw here: [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-FIXES: void InClass() const {
+
+
+void forward_declared() noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: In function declared no-throw here: [misc-throw-with-noexcept]
+// CHECK-FIXES: void forward_declared() ;
+
+void forward_declared() noexcept {
+ throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:25: warning: In function declared no-throw here: [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-3]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-FIXES: void forward_declared() {
+
+#define NOEXCEPT noexcept
+
+void with_macro() NOEXCEPT {
+ throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+
+void dynamic_exception_spec() throw() {
+ throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// FIXME expect a fix here
+
+void getFunction() noexcept {
+ struct X {
+ static void inner()
+ {
+ throw 42;
+ }
+ };
+}
Index: docs/clang-tidy/checks/misc-throw-with-noexcept.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-throw-with-noexcept.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - misc-throw-with-noexcept
+
+boost-use-to-string
+===================
+
+This check finds cases of using ``throw`` in a function declared as noexcept.
+Please note that the warning is issued even if the exception is caught within
+the same function, as that would be probably a bad style anyway.
+
+It removes the noexcept specifier as a fix.
+
+
+ .. code-block:: c++
+
+ void f() noexcept {
+ throw 42;
+ }
+
+ // Will be changed to
+ void f() {
+ throw 42;
+ }
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -83,6 +83,7 @@
misc-suspicious-string-compare
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
+ misc-throw-with-noexcept
misc-unconventional-assign-operator
misc-undelegated-constructor
misc-uniqueptr-reset-release
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -63,9 +63,14 @@
explain them more clearly, and provide more accurate fix-its for the issues
identified. The improvements since the 3.8 release include:
+- New `misc-throw-with-noexcept
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-throw-with-noexcept.html>`_ check
+
+ Flags ``throw`` statements in functions marked as no-throw.
+
- New Boost module containing checks for issues with Boost library.
-- New `boost-use-to-string
+- New `boost-use-to-string
<http://clang.llvm.org/extra/clang-tidy/checks/boost-use-to-string.html>`_ check
Finds usages of ``boost::lexical_cast<std::string>`` and changes it to
Index: clang-tidy/misc/ThrowWithNoexceptCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/ThrowWithNoexceptCheck.h
@@ -0,0 +1,33 @@
+//===--- ThrowWithNoexceptCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_WITH_NOEXCEPT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_WITH_NOEXCEPT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+///\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 {
+public:
+ ThrowWithNoexceptCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_WITH_NOEXCEPT_H
Index: clang-tidy/misc/ThrowWithNoexceptCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/ThrowWithNoexceptCheck.cpp
@@ -0,0 +1,157 @@
+//===--- ThrowWithNoexceptCheck.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 "ThrowWithNoexceptCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void ThrowWithNoexceptCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus11)
+ return;
+ Finder->addMatcher(
+ cxxThrowExpr(stmt(forFunction(functionDecl(isNoThrow()).bind("func"))))
+ .bind("throw"),
+ this);
+}
+
+// FIXME: Move to utils as it is used in multiple checks.
+// Looks for the token matching the predicate and returns the range of the found
+// token including trailing whitespace.
+static SourceRange findToken(const SourceManager &Sources, LangOptions LangOpts,
+ SourceLocation StartLoc, SourceLocation EndLoc,
+ bool (*Pred)(const Token &)) {
+ if (StartLoc.isMacroID() || EndLoc.isMacroID())
+ return SourceRange();
+ FileID File = Sources.getFileID(Sources.getSpellingLoc(StartLoc));
+ StringRef Buf = Sources.getBufferData(File);
+ const char *StartChar = Sources.getCharacterData(StartLoc);
+ Lexer Lex(StartLoc, LangOpts, StartChar, StartChar, Buf.end());
+ Lex.SetCommentRetentionState(true);
+ Token Tok;
+ do {
+ Lex.LexFromRawLexer(Tok);
+ if (Pred(Tok)) {
+ Token NextTok;
+ Lex.LexFromRawLexer(NextTok);
+ return SourceRange(Tok.getLocation(), NextTok.getLocation());
+ }
+ } while (Tok.isNot(tok::eof) && Tok.getLocation() < EndLoc);
+
+ return SourceRange();
+}
+
+// Finds expression enclosed in parentheses, starting at StartLoc.
+// Returns the range of the expression including trailing whitespace.
+static SourceRange parenExprRange(const SourceManager &Sources,
+ LangOptions LangOpts, SourceLocation StartLoc,
+ SourceLocation EndLoc) {
+ if (StartLoc.isMacroID() || EndLoc.isMacroID())
+ return SourceRange();
+ FileID File = Sources.getFileID(Sources.getSpellingLoc(StartLoc));
+ StringRef Buf = Sources.getBufferData(File);
+ const char *StartChar = Sources.getCharacterData(StartLoc);
+ Lexer Lex(StartLoc, LangOpts, StartChar, StartChar, Buf.end());
+ Lex.SetCommentRetentionState(true);
+ Token Tok;
+ Lex.LexFromRawLexer(Tok);
+ assert(Tok.is(tok::l_paren));
+ int OpenParenCount = 1;
+ do {
+ Lex.LexFromRawLexer(Tok);
+ if (Tok.is(tok::r_paren)) {
+ --OpenParenCount;
+ if (OpenParenCount == 0) {
+ Token NextTok;
+ Lex.LexFromRawLexer(NextTok);
+ return SourceRange(StartLoc, NextTok.getLocation());
+ }
+ } else if (Tok.is(tok::l_paren)) {
+ ++OpenParenCount;
+ }
+ } while (Tok.isNot(tok::eof) && Tok.getLocation() < EndLoc);
+
+ return SourceRange();
+}
+
+static SourceRange findNoExceptRange(const ASTContext &Context,
+ const SourceManager &Sources,
+ const FunctionDecl &Function) {
+ // FIXME: Support dynamic exception specification (e.g. throw())
+ /* Find noexcept token */
+ auto isKWNoexcept = [](const Token &Tok) {
+ return Tok.is(tok::raw_identifier) && Tok.getRawIdentifier() == "noexcept";
+ };
+ SourceRange NoExceptTokenRange =
+ findToken(Sources, Context.getLangOpts(), Function.getOuterLocStart(),
+ Function.getLocEnd(), isKWNoexcept);
+
+ if (!NoExceptTokenRange.isValid())
+ return SourceRange();
+
+ const auto *ProtoType = Function.getType()->getAs<FunctionProtoType>();
+ const auto NoexceptSpec = ProtoType->getNoexceptSpec(Context);
+ if (NoexceptSpec != FunctionProtoType::NR_Nothrow) {
+ /* We have noexcept that doesn't evaluate to true,
+ * something strange or malformed. */
+ return SourceRange();
+ } else if (ProtoType->getNoexceptExpr() == nullptr) {
+ /* We have noexcept without the expression inside */
+ return NoExceptTokenRange;
+ }
+
+ /* Now we have to merge */
+ SourceRange ExprRange =
+ parenExprRange(Sources, Context.getLangOpts(),
+ NoExceptTokenRange.getEnd(), Function.getLocEnd());
+
+ if (!ExprRange.isValid())
+ return SourceRange();
+
+ return SourceRange(NoExceptTokenRange.getBegin(), ExprRange.getEnd());
+}
+
+void ThrowWithNoexceptCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ const auto *Throw = Result.Nodes.getNodeAs<Stmt>("throw");
+ diag(Throw->getLocStart(), "'throw' expression in a function declared with a "
+ "non-throwing exception specification");
+
+ llvm::SmallVector<SourceRange, 5> NoExceptRanges;
+ for (const auto *Redecl : Function->redecls()) {
+ SourceRange NoExceptRange =
+ findNoExceptRange(*Result.Context, *Result.SourceManager, *Redecl);
+
+ if (NoExceptRange.isValid()) {
+ NoExceptRanges.push_back(NoExceptRange);
+ } else {
+ /* If a single one is not valid, we cannot apply the fix as we need to
+ * remove noexcept in all declarations for the fix to be effective. */
+ NoExceptRanges.clear();
+ break;
+ }
+ }
+
+ for (const auto &NoExceptRange : NoExceptRanges) {
+ // FIXME use DiagnosticIDs::Level::Note
+ diag(NoExceptRange.getBegin(), "In function declared no-throw here:")
+ << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ NoExceptRange.getBegin(), NoExceptRange.getEnd()));
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -43,6 +43,7 @@
#include "SuspiciousStringCompareCheck.h"
#include "SwappedArgumentsCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
+#include "ThrowWithNoexceptCheck.h"
#include "UndelegatedConstructor.h"
#include "UniqueptrResetReleaseCheck.h"
#include "UnusedAliasDeclsCheck.h"
@@ -122,6 +123,8 @@
"misc-swapped-arguments");
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
"misc-throw-by-value-catch-by-reference");
+ CheckFactories.registerCheck<ThrowWithNoexceptCheck>(
+ "misc-throw-with-noexcept");
CheckFactories.registerCheck<UndelegatedConstructorCheck>(
"misc-undelegated-constructor");
CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -35,6 +35,7 @@
SuspiciousStringCompareCheck.cpp
SwappedArgumentsCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
+ ThrowWithNoexceptCheck.cpp
UndelegatedConstructor.cpp
UniqueptrResetReleaseCheck.cpp
UnusedAliasDeclsCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
