clang-tooling: replacing out parameter with a return value

Question

I'm writing a clang tool that would roughly do the following:

Before:

encode(in, out);

After:

out = encode(in);

The problem is I would like to remove the unnecessary temporary variables too.

At the very least I'd like:

std::string test(std::string_view in) {
  std::string res;
  encode(in, res);
  return res;
}

To be replaced with:

std::string test(std::string_view in) {
  return encode(in);
}

All ideas I have about how to do this are fairly involved, I was wondering if someone has a reasonable answer.

Would `auto res = encode(in); return res;` satisfy you? There are some advantages to this, when debugging. — Paul Sanders, Nov 27 '22 at 14:34
I'd prefer to completly remove the temp. But if you know a good way to do your way - I am listening, there might be some way to advance it. — Denis Yaroshevskiy, Nov 27 '22 at 18:28
Sorry, I don't use clang tidy, I was just making what I thought might be a helpful suggestion. — Paul Sanders, Nov 27 '22 at 22:35

Denis Yaroshevskiy · Accepted Answer · 2022-12-07T15:42:22.677

NOTE: refactoring I got is fairly at most good-enough for my needs and is likely to break under some non obvious conditions. I intend to code-review the results.

No smart way was discovered, posting what I got. Unfortunately my whole code is closed sourced, so I can't post entire solution.

The main thing I didn't know about the solution is that you can run a matcher on a subtree.

Here is a function that returns all of the usages for a given declaration.

std::vector<const clang::DeclRefExpr*> findDeclRefs(
    const clang::CompoundStmt* parent,
    const clang::Decl* decl,
    clang::ASTContext& context) {
  using namespace clang::ast_matchers;
  auto matches = match(
      findAll(declRefExpr(hasDeclaration(equalsNode(decl))).bind("usage")),
      *parent,
      context);

  std::vector<const clang::DeclRefExpr*> res;
  for (const auto& match : matches) {
    res.push_back(match.getNodeAs<clang::DeclRefExpr>("usage"));
  }

  return res;
}

Here is a function that returns last usage of the out parameter if it's appropriate to replace it (for my case). This is where most of your modifications are likely to be.

const clang::DeclRefExpr* canOutParameterBeInlined(
    const clang::CompoundStmt* scope,
    const clang::Expr* param,
    clang::ASTContext& context) {
  using namespace clang::ast_matchers;
  auto* paramVar = llvm::dyn_cast_or_null<clang::DeclRefExpr>(param);

  // Out parameter is not just reference but some complicated expression.
  if (paramVar == nullptr) {
    return nullptr;
  }

  auto* decl = paramVar->getDecl();

  // Out param was declared outside of the scope
  if (match(compoundStmt(hasDescendant(equalsNode(decl))), *scope, context)
          .empty()) {
    return nullptr;
  }

  auto usages = findDeclRefs(scope, decl, context);


  // In order to replace, we should have exactly 2 usages:
  // in the call and in the result.
  if (usages.size() != 2U) {
    return nullptr;
  }

  // shouldn't happen: one usage gotta be the call
  if (usages[0] != paramVar && usages[1] != paramVar) {
    return nullptr;
  }

  // One usage gotta be not the call
  if (usages[0] != paramVar) {
    return usages[0];
  }

  if (usages[1] != paramVar) {
    return usages[1];
  }

  return nullptr;
}

PS: I also struggled to remove semicolons. The function I was looking for is: clang::Lexer::findLocationAfterToken(end, clang::tok::semi, ...)

clang-tooling: replacing out parameter with a return value

1 Answers1