[clang-tools-extra] r312134 - [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 30 09:27:49 PDT 2017
On Wed, Aug 30, 2017 at 12:16 PM, Jonas Toth via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> 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.
Many of the builders will send an email directly to all of the authors
of commits that were pulled and caused the build to fail, but not all
of them. You may want to check your spam filters as well, because
sometimes the build failure emails wind up there. The bots will also
spam the IRC channel when they go red or green. My recommendation is
to be in the IRC channel whenever you make a commit and watch to see
if any bots go red with your commit handle in the blame list.
If the issue is minor and just needs a small tweak to correct, just go
ahead and commit the fix to get the bots back to green. But if you
want more time to address the issue, or it turns out to be something
other than a simple fix, we usually suggest folks revert their commit
entirely and re-commit later once the issue has been addressed.
Basically, the goal is to get the bots back to green quickly.
HTH!
~Aaron
>
> 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
>
>
> _______________________________________________
> 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