[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. In D55433#1357483 , @MyDeveloperDay wrote: > In D55433#1351707 , @JonasToth wrote: > > > > I do not have commit rights. I'm not sure what constitutes someone who > > > can commit, but le

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1351707 , @JonasToth wrote: > > I do not have commit rights. I'm not sure what constitutes someone who can > > commit, but let me contribute a little more before taking that step, I > > have another idea for a c

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

This revision was automatically updated to reflect the committed changes. Closed by commit rL350760: [clang-tidy] Adding a new modernize use nodiscard checker (authored by JonasToth, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D5543

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. > I do not have commit rights. I'm not sure what constitutes someone who can > commit, but let me contribute a little more before taking that step, I have > another idea for a checker I'd like to try after this one, I just wanted to > get one under my belt first. Se

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1350999 , @JonasToth wrote: > LGTM! > You verified that your fixes, fix the issues in LLVM? But it looks good to > go. They look good, you asked before... > P.S. did you request commit rights already? I do no

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth accepted this revision. JonasToth added a comment. LGTM! You verified that your fixes, fix the issues in LLVM? But it looks good to go. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 ___ c

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1349709 , @JonasToth wrote: > No problem, thats why we test on real projects first, because there is always > something hidden in C++ :) > > Is there a test for the lambdas? test/clang-tidy/modernize-use-nodis

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180687. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Address review comments - add more lambda tests - add nested lambda test - remove hasParent() call, (seems isConversionOperator()) seems to detect the CXXMethods

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. In D55433#1349709 , @JonasToth wrote: > No problem, thats why we test on real projects first, because there is always > something hidden in C++ :) > > Is there a test for t

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1349709 , @JonasToth wrote: > No problem, thats why we test on real projects first, because there is always > something hidden in C++ :) > > Is there a test for the lambdas? test/clang-tidy/modernize-use-nodis

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. No problem, thats why we test on real projects first, because there is always something hidden in C++ :) Is there a test for the lambdas? Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:39 +AST_MATCHER(CXXMethodDecl, isConversionDecl) { + //

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180625. MyDeveloperDay added a comment. Sorry to add another revision but, rerunning no-discard checker on LLVM code base threw up other [[nodiscard]] placements which whilst they MAY be correct, I don't think we should handle in this first pass. - c

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. In D55433#1347553 , @MyDeveloperDay wrote: > libprotobuf still builds cleanly with just the one expected warning..despite > adding over 500 [[nodiscards]] > > F7800873: image.png > > I'll con

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. libprotobuf still builds cleanly with just the one expected warning..despite adding over 500 [[nodiscards]] F7800873: image.png I'll continue to look at other projects, but I'm happy at present that this gives us a good starti

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth accepted this revision. JonasToth added a comment. LGTM. If you have the time please rerun this latest (final?) version over LLVM or any other bigger project and check if there are any issues left and ensure the code still compiles after code-transformation. CHANGES SINCE LAST ACTIO

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:23 +static bool isAliasedTemplateParamType(QualType ParamType) { + return (ParamType.getCanonicalType()->isTemplateTypeParmType() || +

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180266. MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added a comment. Update with view comments from @JonasToth - remove unnecessary static functions - move checks into matchers - remove duplicated diag call CHANGES SINCE LAST ACTI

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. Mostly nits left. I think the check is good to go afterwards :) Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:23 +static bool isAliasedTemplateParamType(QualType ParamType) { + return (ParamType.getCanonicalType()->isTemplateTypeParmType() |

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30 +// TODO: Find a better way of detecting a function template. +static bool isFunctionTemplate(const QualType &ParamType) { + // Try to catch both std::function and boost::function ---

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180212. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Address review comments - remove using strings to locate std::function and boost::function arguments using matcher with ``hasAnyName()`` instead - add tests to ch

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. let me look further into that, it works for std::function but not for const std::function or std::function &, const std::function & CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 _

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30 +// TODO: Find a better way of detecting a function template. +static bool isFunctionTemplate(const QualType &ParamType) { + // Try to catch both std::function and boost::function

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30 +// TODO: Find a better way of detecting a function template. +static bool isFunctionTemplate(const QualType &ParamType) { + // Try to catch both std::function and boost::function ---

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180073. MyDeveloperDay marked 14 inline comments as done. MyDeveloperDay added a comment. Rebase - update to address most review comments from @JonasToth - simplify matcher (with less unless) - remove the need to look for "type-parameter-" string CHA

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:24 +// parameters via using alias not detected by ``isTemplateTypeParamType()``. +static bool isAliasedTemplateParamType(const QualType &ParamType) { + return (ParamType.getCanonicalType().ge

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. Sorry for my absence the last week, holidays came in between. I will take a look at the code again and try to find a robuster way of template-type detection :) I do have an idea, but not sure if that works as expected (within this week). Sorry again! CHANGES SINCE L

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 179445. MyDeveloperDay added a comment. - Rebase - Add a couple of extra test cases to ensure we don't add the NO_DISCARD macro when a clang or gcc attribute is present - Ping to ask code owners to review and possibly commit if happy. CHANGES SINCE L

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178656. MyDeveloperDay added a comment. Remove unused matcher variable CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidy

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1334150 , @curdeius wrote: > LGTM. > Any ideas for improvement, @JonasToth? > I'd rather have it merged and improve on it later if there are ideas on how > to do it better. Thanks for the LGTM, as you know I d

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178654. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Fix review comments - Fix minor spelling mistakes - add additional boost::function unit test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ ht

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius added a comment. LGTM. Any ideas for improvement, @JonasToth? I'd rather have it merged and improve on it later if there are ideas on how to do it better. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:32 + // Try to catch both std::function and boost::functi

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178367. MyDeveloperDay added a comment. -Refactor the checker to remove the llvm::any_of by using a QualType ast-matcher with hasAnyParameter(hasType(x)) - i.e. ...unless(hasAnyParameter(hasType(isNonConstReferenceOrPointer(... -Reduce the SNR ra

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:23 +Such functions have no means of altering any state or passing values other than +via the return type. Unless the member functi

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178355. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. - Rebase - Reduce false-positives by not adding [[nodiscard]] to functions if the class has mutable fields (removed as known issues) and add tests CHANGES SINCE

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 7 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:25 +static bool isAliasedTemplateParamType(const QualType &ParamType) { + return (ParamType.getCanonicalType().getAsString().find("type-p

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178217. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Fix review comments - minor changes to code comments - add more unit tests for typedef'd template types CHANGES SINCE LAST ACTION https://reviews.llvm.org/D554

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:22 + +// Find a better way of detecting const-reference-template type +// parameters via using alias not detected by ``isTemplateTypeParamType()``. You can write `TODO: ` or `F

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126 +template +class Bar +{ MyDeveloperDay wrote: > MyDeveloperDay wrote: > > JonasToth wrote: > > > MyDeveloperDay wrot

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178123. MyDeveloperDay added a comment. Add additional constraints on member functions - do not add [[nodiscard]] to variadic function - do not add [[nodiscard]] to std::function templates argument functions - do not add [[nodiscard]] to functions retu

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126 +template +class Bar +{ MyDeveloperDay wrote: > JonasToth wrote: > > MyDeveloperDay wrote: > > > curdeius wrote: > >

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178108. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Addressing review comments - don't add [[nodiscard]] onto function taking template arguments - add more unit tests to cover other discard possibilities [[clang::w

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 6 inline comments as done. MyDeveloperDay added a comment. >> >> >> unsigned BlockOrCode = 0; >> llvm::ErrorOr Res = skipUntilRecordOrBlock(Stream, BlockOrCode); >> if (!Res) >> Res.getError(); >> > > AFAIK `llvm::Error` must be consumed because if it goes

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:183 +// CHECK-MESSAGES-NOT: warning: +// CHECK-FIXES: bool f33(T&) const + No warning -> No fix -> You can ellide the `CHECK-FIXES` here and elsewhere. FileCheck

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126 +template +class Bar +{ MyDeveloperDay wrote: > curdeius wrote: > > JonasToth wrote: > > > I think the template tests should be improved. > > > What happens for `T empty(

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. > You also get into trouble because there are many header files that it add > LLVM_NODISCARD to but they don't include "Compiler.h" where LLVM is defined > (so you end up with lots of errors) > 3>C:\llvm\include\llvm/ADT/iterator_range.h(46): error C3646: 'IteratorT'

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. I wanted to leave a comment regarding running the [[nodiscard]] checker over all of clang... as requested by @JonasToth, @lebedev.ri and @Szelethus I'm not 100% sure how to present the results, but let me pick out a few high/low lights... My efforts are somewha

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177679. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Resolving some (not all yet) review comments, and looking for help on template parameter exclusion - add additional template argument tests - add additional clan

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:65 +void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) { + // If we are using C++17 attributes we are going to need c++17 + if (NoDiscardMacro == "[[nodiscard]]") { ---

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius added a comment. A few more ideas for enhancements. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:41 + //bool foo(A*) + // then they may not care about the return value, because of passing data + // via the arguments however functions with no arguments

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:29 +AST_MATCHER(CXXMethodDecl, isOverloadedOperator) { + // Don't put [[nodiscard]] front of operators. + return Node.isOverloadedOperator(); s/front/in front/ ===

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:49 + + +.. code-block:: c++ Unnecessary empty line. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 ___

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177584. MyDeveloperDay marked 11 inline comments as done. MyDeveloperDay added a comment. Addressing review comments - grammatical errors in documentation and comments - prevent adding [[nodiscard]] to macros - clean up list.rst CHANGES SINCE LAST AC

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

JonasToth added a comment. Hi MyDeveloperDay, thanks for the patch! Mostly stylistic comments. Would it make sense to attach the attribute to the implementation of the functions too? This check is definitly useful, but noisy. Do you see a change of another heuristic that could be applied to re

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. @curdeius Thanks, I don't have commit access so I'm happy wait for a CODE_OWNER, they could likely have more input. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 __

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. But I'm not a code owner here and I don't know if you need an acceptance of one of them. Great job. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177532. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Addressing review comments, - additional unit tests for no ReplacementString and C++ 11 case - more expressive hasNonConstReferenceOrPointerArguments matcher - min

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:45 + // + for (const auto *Par : Node.parameters()) { +const Type *ParType = Par->getType().get

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius added a comment. Some more minor remarks. I'll give this check a try to see how it behaves on some of my projects. I agree that a high rate of false positives is possible (and is a bit of spoiler) but I wouldn't reject this IMO useful check because of that. Anyway, everything looks pret

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:37 + +Specify a macro to use instead of ``[[nodiscard]]``. This is useful when +maintaining source code that needs to compile with a pre-C++17 compiler. Option

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177496. MyDeveloperDay added a comment. Update the documentation to utilize 80 character lines CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:37 + +Specify a macro to use instead of ``[[nodiscard]]``. +This is useful when maintaining source code that needs to compile with a pre-C++17 compiler. Specifi

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1325117 , @lebedev.ri wrote: > In D55433#1323779 , @lebedev.ri > wrote: > > > In D55433#1323757 , > > @MyDeveloperDay wrote: > > >

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:62 +Users can use :option:`IgnoreInternalFunctions` to turn off the adding of +``[nodiscard]]`` to functions starting with _ e.g.

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:43 + // function like _Foo() + if (ignore){ + return doesFunctionNameStartWithUnderScore(&Node); curdeius wrote: > If think that you should run clang-format over yo

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177474. MyDeveloperDay marked 11 inline comments as done. MyDeveloperDay added a comment. - Addressing review comments and concerns - Removed internal function logic and option, not really the role of this checker - Fixed grammatical error in documenta

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. In D55433#1323779 , @lebedev.ri wrote: > In D55433#1323757 , @MyDeveloperDay > wrote: > > > a lot of projects aren't setup for c++17 yet which is needed for > > [[nodiscard]] to be allo

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Just a few minor remarks and a possible workaround for testing `CHECK-FIXES: [[nodiscard]]`. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:43 + // fu

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177330. MyDeveloperDay added a comment. Minor alterations to Release notes and Documentation to address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CM

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177327. MyDeveloperDay added a comment. - Move the final conditional statements into AST_MATCHERS - Add additional IgnoreInternalFunctions option to allow adding of ``[[nodiscard]]`` to functions starting with _ (e.g. _Foo()) CHANGES SINCE LAST ACTIO

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:173 + + This check adds ``[[nodiscard]]`` attributes (introduced in C++17) to member + functions to highlight at compile time where the return value of a function Please omit this check. Sa

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177313. MyDeveloperDay added a comment. Move the conditional statements into AST_MATCHERS CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/mode

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177312. MyDeveloperDay added a comment. Move the conditional checks into matchers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/Mo

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177311. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Move more of the conditional checks into the matchers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files:

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 12 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:28-30 +bool isOperator(const FunctionDecl *D) { + return D->getNameAsString().find("operator") != std::string::npos; +}

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

Eugene.Zelenko added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:40 +void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) { + + // if we are using C++17 attributes we are going to need c++17 Unnecessary empty line.

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 177295. MyDeveloperDay added a comment. Address some (not all yet) review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/M

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. In D55433#1323757 , @MyDeveloperDay wrote: > a lot of projects aren't setup for c++17 yet which is needed for > [[nodiscard]] to be allowed, You can use `[[clang::warn_unused_result]]` for this evaluation, that does not req

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. I'm trying to generate an example of the modernize-use-nodiscard checker on something open source...as I developed this checker on Windows I struggled a little getting cmake to build me the json file to run clang-tidy over everything and a lot of projects aren't

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:28-30 +bool isOperator(const FunctionDecl *D) { + return D->getNameAsString().find("operator") != std::string::npos; +} Can't you use `isOverloadedOperator()`? No way going

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

Szelethus added a comment. I recently had trouble with ignoring the return type, I would certainly appreciate a tool like this. Can you test it on LLVM+Clang? Maybe some methods in the AST library could also benefit from `LLVM_NODISCARD` (containers with factories is my no1 thought), and it's a

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. In D55433#1323145 , @MyDeveloperDay wrote: > In D55433#1323106 , @lebedev.ri > wrote: > > > Have you evaluated this on some major C++ projects yet? > > I suspect this may have pretty l

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

Eugene.Zelenko added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:21 +namespace modernize { +namespace { + Please use static instead anonymous namespace for functions. See LLVM code style guidelines. Comment at: clan

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1323106 , @lebedev.ri wrote: > Have you evaluated this on some major C++ projects yet? > I suspect this may have pretty low SNR. Internally yes (on millions of lines), but you are correct high noise level, howe

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. Have you evaluated this on some major C++ projects yet? I suspect this will have pretty low SNR. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: alexfh, stephenkelly, curdeius, aaron.ballman. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Adds a checker to clang-tidy to warn when a non void const member function, taking only parameters passed by value or cons