[clang-tools-extra] r312134 - [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better
Diana Picus via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 30 09:12:18 PDT 2017
Hi Jonas,
I haven't seen any commit from you (*) fixing the buildbot failures
caused by your previous commit (r312122). This has been keeping
several bots red for quite a while:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/10590
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/6845
etc etc
It's customary to keep an eye on the buildbots and either revert or
fix such problems as soon as possible. It's particularly inappropriate
to commit something else on top before dealing with the bot failures.
For the record, Alex has been trying to help you on IRC in #llvm:
"alexlorenz: jonas_toth: btw, char is not signed by default (it's
platform-specific), so you should use 'signed char' in your clang-tidy
test to fix the bot failures"
Please be more careful with the buildbots in the future.
Thanks,
Diana
(*) My apologies if I missed it, but I really don't see anything in
cfe-commits nor in the situation of the bots to reflect that.
On 30 August 2017 at 17:59, Jonas Toth via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> 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
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list