[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 09:16:50 PDT 2017
Hello Diana,
you are right, i did not commit anything earlier, I am really new to the
process, these were my first commits.
I am sorry that i broke the bots, i thought i would get an automatic
mail, when they find a problem and i was in the commit range. Is there
anything automated to find out such issues? I feel a little overwhelmed
with this big amount of builders.
Sorry again, I will look into it.
Regard, Jonas
Am 30.08.2017 um 18:12 schrieb Diana Picus:
> 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