[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

shafik added a comment. Herald added a project: All. Is this PR still relevant or can we close it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19201/new/ https://reviews.llvm.org/D19201 ___ cfe-commits mailing list

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski added a comment. In https://reviews.llvm.org/D19201#768693, @baloghadamsoftware wrote: > There is a patch (https://reviews.llvm.org/D33537) for a check which is a > superset of this: [...] A more important difference is that we traverse the > whole call-chain and check all the throw

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

baloghadamsoftware added a comment. There is a patch (https://reviews.llvm.org/D33537) for a check which is a superset of this: It does not only check for noexcept (and throw()) functions, but also for destructors, move constructors, move assignments, the main() function, swap() functions and

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

aaron.ballman added a comment. In https://reviews.llvm.org/D19201#760358, @Prazek wrote: > In https://reviews.llvm.org/D19201#760151, @aaron.ballman wrote: > > > As an FYI, there is a related check being implemented in clang currently; > > we probably should not duplicate this effort. See > >

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek accepted this revision. Prazek added a comment. LGTM, but wait if Aaron will accept it. https://reviews.llvm.org/D19201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added a comment. In https://reviews.llvm.org/D19201#760151, @aaron.ballman wrote: > As an FYI, there is a related check being implemented in clang currently; we > probably should not duplicate this effort. See > https://reviews.llvm.org/D3. I think that clang is the right place

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski marked 3 inline comments as done. sbarzowski added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:25 + Finder->addMatcher( + cxxThrowExpr(stmt(hasAncestor(functionDecl(isNoThrow()).bind("func" + .bind("throw"),

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski updated this revision to Diff 99670. sbarzowski marked 5 inline comments as done. sbarzowski added a comment. Cosmetic https://reviews.llvm.org/D19201 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/ThrowWithNoexceptCheck.cpp

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

aaron.ballman added a comment. As an FYI, there is a related check being implemented in clang currently; we probably should not duplicate this effort. See https://reviews.llvm.org/D3. https://reviews.llvm.org/D19201 ___ cfe-commits mailing

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: docs/ReleaseNotes.rst:58 +- New `misc-throw-with-noexcept + `_ check I think this should be in alphabetical order.

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:76 + + for (const auto Parent : Context->getParents(node)) { +if (isCaughtInFunction(Context, Throw, Function, Parent)) unnecessary braces Comment at:

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski updated this revision to Diff 99502. sbarzowski marked 8 inline comments as done. sbarzowski added a comment. Removed unnecessary colon from message https://reviews.llvm.org/D19201 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski updated this revision to Diff 99500. sbarzowski marked 3 inline comments as done. sbarzowski added a comment. Docs and cosmetic issues https://reviews.llvm.org/D19201 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:22 +void ThrowWithNoexceptCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus11) +return; If we handle throw() then it should be CPlusPlus

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski updated this revision to Diff 97859. sbarzowski added a comment. Herald added a subscriber: xazax.hun. Fixed false positive issues https://reviews.llvm.org/D19201 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

aaron.ballman added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20 +///\brief Warns about using throw in function declared as noexcept. +/// It complains about every throw, even if it is caught later. +class ThrowWithNoexceptCheck : public

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20 +///\brief Warns about using throw in function declared as noexcept. +/// It complains about every throw, even if it is caught later. +class ThrowWithNoexceptCheck : public ClangTidyCheck {

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20 +///\brief Warns about using throw in function declared as noexcept. +/// It complains about every throw, even if it is caught later. +class ThrowWithNoexceptCheck : public ClangTidyCheck {

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

vmiklos added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20 +///\brief Warns about using throw in function declared as noexcept. +/// It complains about every throw, even if it is caught later. +class ThrowWithNoexceptCheck : public ClangTidyCheck {

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54 +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "in a function declared no-throw here:", DiagnosticIDs::Note) +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. A couple of nits. Please address Aaron's comment as well. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:53 + for (const auto : NoExceptRanges) { +//

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski updated this revision to Diff 85882. sbarzowski marked 3 inline comments as done. sbarzowski added a comment. Improved messages, added tests with templates, fixed some typos. https://reviews.llvm.org/D19201 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:44 +} else { + /* If a single one is not valid, we cannot apply the fix as we need to + *

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:52-53 + for (const auto : NoExceptRanges) { +// FIXME use DiagnosticIDs::Level::Note +diag(NoExceptRange.getBegin(), "In function declared no-throw here:") +<<

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski removed rL LLVM as the repository for this revision. sbarzowski updated this revision to Diff 84577. sbarzowski added a comment. Herald added subscribers: JDevlieghere, mgorny. I took advantage of new getExceptionSpecSourceRange (it wasn't available before) instead of getting

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Removing from my dashboard until http://reviews.llvm.org/D20428 is submitted. Repository: rL LLVM http://reviews.llvm.org/D19201

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski marked 11 inline comments as done. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:33 @@ +32,3 @@ +// token including trailing whitespace. +static SourceRange findToken(const SourceManager , LangOptions LangOpts, + SourceLocation

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:25 @@ +24,3 @@ + Finder->addMatcher( + cxxThrowExpr(stmt(hasAncestor(functionDecl(isNoThrow()).bind("func" + .bind("throw"), aaron.ballman wrote: > Prazek

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski updated this revision to Diff 59277. sbarzowski added a comment. Thanks for review, guys. I fixed as many easy problems as I had time for today, will continue later. Repository: rL LLVM http://reviews.llvm.org/D19201 Files: clang-tidy/misc/CMakeLists.txt

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

aaron.ballman requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:25 @@ +24,3 @@ + Finder->addMatcher( + cxxThrowExpr(stmt(hasAncestor(functionDecl(isNoThrow()).bind("func" +

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:32 @@ +31,3 @@ +// token including trailing whitespace. +static SourceRange FindToken(const SourceManager , LangOptions LangOpts, + SourceLocation StartLoc,

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added inline comments. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:25 @@ +24,3 @@ + Finder->addMatcher( + cxxThrowExpr(stmt(hasAncestor(functionDecl(isNoThrow()).bind("func" + .bind("throw"), you can use forFunction instead of

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

Prazek added a comment. In http://reviews.llvm.org/D19201#445496, @aaron.ballman wrote: > In http://reviews.llvm.org/D19201#445406, @sbarzowski wrote: > > > Note ``FunctionProtoType::getNoExceptExpr`` is weird. If you have the same > > expr in multiple noexcepts, then it returns the same object

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

aaron.ballman added a subscriber: aaron.ballman. aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. In http://reviews.llvm.org/D19201#445406, @sbarzowski wrote: > Note ``FunctionProtoType::getNoExceptExpr`` is weird. If you have the same > expr in multiple noexcepts,

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski added a comment. Note ``FunctionProtoType::getNoExceptExpr`` is weird. If you have the same expr in multiple noexcepts, then it returns the same object for all of them, so it is useless for determining the location in code. I ended up finding it all "by hand" with lexer (looking for

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

sbarzowski updated the summary for this revision. sbarzowski removed rL LLVM as the repository for this revision. sbarzowski updated this revision to Diff 59179. http://reviews.llvm.org/D19201 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Please fix formatting, btw. Repository: rL LLVM http://reviews.llvm.org/D19201 ___ cfe-commits mailing list

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh added a comment. In http://reviews.llvm.org/D19201#403585, @Prazek wrote: > Do you know guys is it possible to get to noexcept source location, or we > have to do by hand using lexer? If it might be possible to get the location of `noexcept(expression)` using