[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:38:04 PDT 2017
On 30 August 2017 at 18:29, Jonas Toth <jonas.toth at gmail.com> wrote:
> Hi Diana,
>
> i will investigate this issue. Can i see from somewhere, what email my
> buildbot uses?
I actually don't know if you can check anywhere, but in principle it
should be the same address that the commit email came from (i.e. this
one, that we're using to talk).
> To fix the break I created a new Patch https://reviews.llvm.org/D37301
>
> What is the right thing now, wait for review or just commit it and that
> review would occur on the commit?
Aaron explained this up-thread much more clearly than I would, thanks Aaron! :)
> Thank you for your help and patience.
>
>
> Regard Jonas
>
>
>
> Am 30.08.2017 um 18:25 schrieb Diana Picus:
>>
>> Hi Jonas,
>>
>> Usually the buildbots send emails when you're in the commit range. I'm
>> surprised that you haven't received anything. Are you sure you don't
>> have some mail filters that are catching them and sending them
>> somewhere unexpected?
>>
>> In any case, you can see a live-but-slow buildbot status here:
>> http://lab.llvm.org:8011/console
>> I usually keep an eye on that when I commit something.
>>
>> We also have the #llvm and #llvm-build channels where the buildbots
>> post reports of failed builds and who is in the commit range, so you
>> should see those unless you're specifically ignoring the
>> llvmbb-llvm-build, bb9-chapuni, bb-pgr and green-dragon-bot users.
>>
>> Hope that helps,
>> Diana
>>
>> On 30 August 2017 at 18:16, Jonas Toth <jonas.toth at gmail.com> 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.
>>>
>>> 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