[cfe-dev] [clang-tidy] Adding a new use nodiscard checker (help needed)

[Curdeius Curdeius via cfe-dev]

Hi,

That may be a really nice check indeed.
I think that a good way to proceed would be to start a review on
Phabricator (https://reviews.llvm.org/).

>From what I've seen there are some things that you could achieve through
AST matchers instead of checking conditions when already having a candidate
match.

For instance, instead of:
Finder->addMatcher(
      cxxMethodDecl(eachOf(unless(returns(voidType())), (isConst())))
          .bind("noDiscardCandidate"),
      this);
and then  if (MatchedDecl->hasUnusedResultAttr())

you can just add unless(hasAttr(clang::attr::WarnUnusedResult)) inside
eachOf.

And for the hint insertion location, these 3 should always be fine for you
even in the case of trailing return type:
* "getBeginLoc()"
* "getInnerLocStart()"
* "getOuterLocStart()"
See slightly modified Stephen's example at:
http://ec2-52-14-16-249.us-east-2.compute.amazonaws.com:10240/z/8GsU8c.

Best regards,
Marek


> ---------- Forwarded message ----------
> From: MyDeveloper Day via cfe-dev <cfe-dev at lists.llvm.org>
> To:
> Cc: cfe-dev at lists.llvm.org
> Bcc:
> Date: Thu, 6 Dec 2018 20:13:07 +0000
> Subject: Re: [cfe-dev] [clang-tidy] Adding a new use nodiscard checker
> (help needed)
> Thanks for the pointers.. this led me to look a little deeper at some of
> the other checkers
>
> looks like ConstReturnTypeCheck uses getInnerLocStart() to locate the
> return type
>
> Substituting this in seems to find the correct location, but I suspect
> this doesn't work well for the training return type
>
> auto retLoc = MatchedDecl->getInnerLocStart();
>
>   // This function could be marked [[nodiscard]]
>   diag(retLoc, "function %0 should be marked [[nodiscard]]")
>       << MatchedDecl << FixItHint::CreateInsertion(retLoc, "[[nodiscard]]
> ");
>
>
> ---------------------------------------------------------------
>
> test.cxx:18:5: warning: function 'empty' should be marked [[nodiscard]]
> [modernize-use-nodiscard]
>     bool empty() const
>     ^
>     [[nodiscard]]
> test.cxx:22:5: warning: function 'empty' should be marked [[nodiscard]]
> [modernize-use-nodiscard]
>     bool empty(int val) const
>     ^
>     [[nodiscard]]
> test.cxx:50:5: warning: function 'empty' should be marked [[nodiscard]]
> [modernize-use-nodiscard]
>     const bool empty() const
>     ^
>     [[nodiscard]]
> test.cxx:55:5: warning: function 'empty' should be marked [[nodiscard]]
> [modernize-use-nodiscard]
>     inline const bool empty() const
>     ^
>     [[nodiscard]]
>
> On Thu, Dec 6, 2018 at 7:19 PM Stephen Kelly via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> On 06/12/2018 19:08, MyDeveloper Day via cfe-dev wrote:
>> > I'm currently using (following a series of trail and errors) to find
>> the
>> > location just before the return type
>>
>> Here are the locations you can get for FunctionDecls:
>>
>>   http://ec2-52-14-16-249.us-east-2.compute.amazonaws.com:10240/z/oiG2nf
>>
>> See also if you have not already:
>>
>>
>>
>> https://steveire.wordpress.com/2018/11/11/future-developments-in-clang-query/
>>
>> If there isn't a location in that output for what you need, then you are
>> going to have to check the tokens yourself. Many existing clang tidy
>> checks do things like this due to lack of location for a particular need.
>>
>> Thanks,
>>
>> Stephen.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181207/a94889e6/attachment.html>