[clang-tools-extra] r312134 - [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better
Jonas Toth via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 30 08:59:01 PDT 2017
Author: jonastoth
Date: Wed Aug 30 08:59:01 2017
New Revision: 312134
URL: http://llvm.org/viewvc/llvm-project?rev=312134&view=rev
Log:
[clang-tidy] Improve hicpp-exception-baseclass to handle generic code better
Summary:
This patch is a followup to the first revision D36583, that had problems with
generic code and its diagnostic messages, which were found by @lebedev.ri
Reviewers: alexfh, aaron.ballman, lebedev.ri, hokein
Reviewed By: aaron.ballman, lebedev.ri
Subscribers: klimek, sbenza, cfe-commits, JDevlieghere, lebedev.ri, xazax.hun
Differential Revision: https://reviews.llvm.org/D37060
Modified:
clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp
Modified: clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp?rev=312134&r1=312133&r2=312134&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp Wed Aug 30 08:59:01 2017
@@ -11,8 +11,6 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include <iostream>
-
using namespace clang::ast_matchers;
namespace clang {
@@ -24,20 +22,23 @@ void ExceptionBaseclassCheck::registerMa
return;
Finder->addMatcher(
- cxxThrowExpr(
- allOf(
- has(expr(unless(hasType(cxxRecordDecl(
- isSameOrDerivedFrom(hasName("std::exception"))))))),
- eachOf(has(expr(hasType(namedDecl().bind("decl")))), anything())))
+ cxxThrowExpr(allOf(has(expr(unless(hasType(qualType(hasCanonicalType(
+ hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
+ hasName("std::exception")))))))))),
+ has(expr(unless(cxxUnresolvedConstructExpr()))),
+ eachOf(has(expr(hasType(namedDecl().bind("decl")))),
+ anything())))
.bind("bad_throw"),
this);
}
void ExceptionBaseclassCheck::check(const MatchFinder::MatchResult &Result) {
const auto *BadThrow = Result.Nodes.getNodeAs<CXXThrowExpr>("bad_throw");
- diag(BadThrow->getLocStart(),
- "throwing an exception whose type is not derived from 'std::exception'")
- << BadThrow->getSourceRange();
+
+ diag(BadThrow->getSubExpr()->getLocStart(), "throwing an exception whose "
+ "type %0 is not derived from "
+ "'std::exception'")
+ << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();
const auto *TypeDecl = Result.Nodes.getNodeAs<NamedDecl>("decl");
if (TypeDecl != nullptr)
Modified: clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp?rev=312134&r1=312133&r2=312134&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/hicpp-exception-baseclass.cpp Wed Aug 30 08:59:01 2017
@@ -5,38 +5,175 @@ class exception {};
} // namespace std
class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
class non_derived_exception {};
+class terrible_idea : public non_derived_exception, public derived_exception {};
+
+// FIXME: More complicated kinds of inheritance should be checked later, but there is
+// currently no way use ASTMatchers for this kind of task.
+#if 0
+class bad_inheritance : private std::exception {};
+class no_good_inheritance : protected std::exception {};
+class really_creative : public non_derived_exception, private std::exception {};
+#endif
void problematic() {
try {
- throw int(42); // Built in is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
+ throw int(42);
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
} catch (int e) {
}
- throw int(42); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
+ throw int(42);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+
+ try {
+ throw 12;
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ } catch (...) {
+ throw; // Ok, even if the type is not known, conforming code can never rethrow a non-std::exception object.
+ }
try {
- throw non_derived_exception(); // Some class is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+ throw non_derived_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 9:1: note: type defined here
} catch (non_derived_exception &e) {
}
- throw non_derived_exception(); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+ throw non_derived_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 9:1: note: type defined here
+
+// FIXME: More complicated kinds of inheritance should be checked later, but there is
+// currently no way use ASTMatchers for this kind of task.
+#if 0
+ // Handle private inheritance cases correctly.
+ try {
+ throw bad_inheritance();
+ // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+ // CHECK MESSAGES: 10:1: note: type defined here
+ throw no_good_inheritance();
+ // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+ // CHECK MESSAGES: 11:1: note: type defined here
+ throw really_creative();
+ // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+ // CHECK MESSAGES: 12:1: note: type defined here
+ } catch (...) {
+ }
+ throw bad_inheritance();
+ // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+ // CHECK MESSAGES: 10:1: note: type defined here
+ throw no_good_inheritance();
+ // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+ // CHECK MESSAGES: 11:1: note: type defined here
+ throw really_creative();
+ // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+ // CHECK MESSAGES: 12:1: note: type defined here
+#endif
}
void allowed_throws() {
try {
- throw std::exception(); // Ok
+ throw std::exception(); // Ok
} catch (std::exception &e) { // Ok
}
throw std::exception();
try {
- throw derived_exception(); // Ok
+ throw derived_exception(); // Ok
} catch (derived_exception &e) { // Ok
}
throw derived_exception(); // Ok
+
+ try {
+ throw deep_hierarchy(); // Ok, multiple levels of inheritance
+ } catch (deep_hierarchy &e) { // Ok
+ }
+ throw deep_hierarchy(); // Ok
+
+ try {
+ throw terrible_idea(); // Ok, but multiple inheritance isn't clean
+ } catch (std::exception &e) { // Can be caught as std::exception, even with multiple inheritance
+ }
+ throw terrible_idea(); // Ok, but multiple inheritance
+}
+
+// Templated function that throws exception based on template type
+template <typename T>
+void ThrowException() { throw T(); }
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 120:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 120:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 123:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
+#define THROW_EXCEPTION(CLASS) ThrowException<CLASS>()
+#define THROW_BAD_EXCEPTION throw int(42);
+#define THROW_GOOD_EXCEPTION throw std::exception();
+#define THROW_DERIVED_EXCEPTION throw deep_hierarchy();
+
+template <typename T>
+class generic_exception : std::exception {};
+
+template <typename T>
+class bad_generic_exception {};
+
+template <typename T>
+class exotic_exception : public T {};
+
+void generic_exceptions() {
+ THROW_EXCEPTION(int);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ THROW_EXCEPTION(non_derived_exception);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK MESSAGES: 9:1: note: type defined here
+ THROW_EXCEPTION(std::exception); // Ok
+ THROW_EXCEPTION(derived_exception); // Ok
+ THROW_EXCEPTION(deep_hierarchy); // Ok
+
+ THROW_BAD_EXCEPTION;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ // CHECK-MESSAGES: [[@LINE-25]]:35: note: expanded from macro 'THROW_BAD_EXCEPTION'
+ THROW_GOOD_EXCEPTION;
+ THROW_DERIVED_EXCEPTION;
+
+ throw generic_exception<int>(); // Ok,
+ THROW_EXCEPTION(generic_exception<float>); // Ok
+
+ throw bad_generic_exception<int>();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+ throw bad_generic_exception<std::exception>();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+ THROW_EXCEPTION(bad_generic_exception<int>);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+ THROW_EXCEPTION(bad_generic_exception<std::exception>);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+
+ throw exotic_exception<non_derived_exception>();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+ THROW_EXCEPTION(exotic_exception<non_derived_exception>);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+
+ throw exotic_exception<derived_exception>(); // Ok
+ THROW_EXCEPTION(exotic_exception<derived_exception>); // Ok
+}
+
+// Test for typedefed exception types
+typedef int TypedefedBad;
+typedef derived_exception TypedefedGood;
+using UsingBad = int;
+using UsingGood = deep_hierarchy;
+
+void typedefed() {
+ throw TypedefedBad();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'TypedefedBad' (aka 'int') is not derived from 'std::exception'
+ // CHECK-MESSAGES: 164:1: note: type defined here
+ throw TypedefedGood(); // Ok
+
+ throw UsingBad();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'UsingBad' (aka 'int') is not derived from 'std::exception'
+ // CHECK-MESSAGES: 166:1: note: type defined here
+ throw UsingGood(); // Ok
}
More information about the cfe-commits
mailing list