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

[MyDeveloper Day via cfe-dev]

I'm trying to write a "use [[nodiscard]]" clang-tidy checker/fix-it to
advise when a function might benefit from having [[nodiscard]] added

Mostly this is working working except I'm finding the wrong location for
the place to put the [[nodiscard]] for functions which have const,inline or
virtual kewords infront of the return type.

e.g.

test.cxx:18:5: warning: function ' empty ' should be made [[nodiscard]]
[modernize-use-nodiscard]
    bool empty() const
    ^
    [[nodiscard]]
test.cxx:26:5: warning: function ' empty' should be made [[nodiscard]]
[modernize-use-nodiscard]
    bool  empty(const A &val) const
    ^
    [[nodiscard]]
test.cxx:50:11: warning: function ' empty' should be made [[nodiscard]]
[modernize-use-nodiscard]
    const bool  empty() const
          ^
          [[nodiscard]]
test.cxx:55:18: warning: function ' empty ' should be made [[nodiscard]]
[modernize-use-nodiscard]
    inline const bool  empty() const
                 ^
                 [[nodiscard]]

And some compilers don't like the [[nodiscard] being placed in between
those keywords and the return type (preferring them to be at the front of
the function)

I'm currently using (following a series of trail and errors) to find the
location just before the return type
MatchedDecl->getReturnTypeSourceRange().getBegin()

e.g.

// This function could be marked [[nodiscard]]
  diag(MatchedDecl->getReturnTypeSourceRange().getBegin(),
       "function %0 should be marked [[nodiscard]]")
      << MatchedDecl
      << FixItHint::CreateInsertion(
             MatchedDecl->getReturnTypeSourceRange().getBegin(),
             "[[nodiscard]] ");


Does anyone know how I should correctly identify the position in front of
all the keywords(virtual,inline,const)?

Especially in the case where the return type is on a different source code
line

e.g.
virtual
const bool full()

Many thanks in advance, feel free to point out any other "errors of my
ways" as this is my first ever checker.

MyDeveloperDay

a current copy of the current checker code is presented here for
completeness
------------------------------------------------------------------------------
namespace clang {
namespace tidy {
namespace modernize {

void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) {

  Finder->addMatcher(
      cxxMethodDecl(eachOf(unless(returns(voidType())), (isConst())))
          .bind("noDiscardCandidate"),
      this);
}

static bool isNonConstReferenceType(QualType ParamType) {
  return ParamType->isReferenceType() &&
         !ParamType.getNonReferenceType().isConstQualified();
}

static bool isOperator(const FunctionDecl *D) {
  return D->getNameAsString().find("operator") != std::string::npos;
}

static bool isInternalFunction(const FunctionDecl *D) {
  return D->getNameAsString().find("_") == 0;
}

void UseNodiscardCheck::check(const MatchFinder::MatchResult &Result) {
  const auto *MatchedDecl =
      Result.Nodes.getNodeAs<CXXMethodDecl>("noDiscardCandidate");

  // if the localtion is invalid forget it
  if (!MatchedDecl->getLocation().isValid())
    return;

  // does it already have [[nodiscard]]
  if (MatchedDecl->hasUnusedResultAttr())
    return;

  // if the function is const it can't be modifying
  // something locally so more likely the result is important
  if (!MatchedDecl->isConst())
    return;

  // if the function is a void or is marked no return then
  // its not a canidate
  if (MatchedDecl->isNoReturn() ||
      MatchedDecl->getReturnType().getAsString() == "void")
    return;

  // don't add [[nodiscard]] to any operators
  if (isOperator(MatchedDecl))
    return;

  // don't add [[nodiscard]] to anything marked as _Foo()
  if (isInternalFunction(MatchedDecl))
    return;

  // if the function has any non constant reference parameters
  // e.g. foo(A &a)
  // the may not care about the return because we may be passing data
  // via the non const reference
  // functions with no arguments will fall through  foo()
  for (const auto *Par : MatchedDecl->parameters()) {
    const Type *ParType = Par->getType().getTypePtr();

    if (isNonConstReferenceType(Par->getType())) {
      return;
    }
  }

  // This function could be marked [[nodiscard]]
  diag(MatchedDecl->getReturnTypeSourceRange().getBegin(),
       "function %0 should be marked [[nodiscard]]")
      << MatchedDecl
      << FixItHint::CreateInsertion(
             MatchedDecl->getReturnTypeSourceRange().getBegin(),
             "[[nodiscard]] ");
  return;
}

} // namespace modernize
} // namespace tidy
} // namespace clang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181206/a1c3476b/attachment.html>