CODEWITHSUNDEEP
c++llvmclang-tidyclang-ast-matchers
Marc Mutz - mmutz
Marc Mutz - mmutz

Reputation: 25293

TransformerClangTidyCheck drops required parentheses around bound `expr()`

I'd like to rename a function on one of the classes:

class Container {
public:
  int count() const; // old
  int size() const; // new
};

So I've written a TransformerClangTidyCheck along the lines of the tutorial (https://clang.llvm.org/docs/ClangTransformerTutorial.html#example-rewriting-method-calls):

ReproParenIssueCheck::ReproParenIssueCheck(llvm::StringRef Name, ClangTidyContext *Context)
    : utils::TransformerClangTidyCheck(Name, Context)
{
    const std::string o = "object";

    auto hasTypeIgnoringPointer = [](auto type) { return anyOf(hasType(type), hasType(pointsTo(type))); };

    auto derivedFromClass = [&](StringRef Class) {
        auto exprOfDeclaredType = [&](auto decl) {
            return hasTypeIgnoringPointer(hasUnqualifiedDesugaredType(recordType(hasDeclaration(decl))));
        };
        return exprOfDeclaredType(cxxRecordDecl(isSameOrDerivedFrom(hasName(Class))));
    };

    auto renameMethod = [&] (StringRef Class, StringRef from, StringRef to) {
        return makeRule(cxxMemberCallExpr(on(expr(derivedFromClass(Class)).bind(o)),
                            callee(cxxMethodDecl(hasName(from), parameterCountIs(0)))),
                        changeTo(cat(access(o, cat(to)), "()")),
                        cat("use '", to, "' instead of '", from, "'"));
    };

    setRule(renameMethod("Container", "count", "size"));
}

This works:

 int test_ok(const Container *p) {
-  return p->count();
+  return p->size();
 }

except that changeTo() is losing the required ParenExpr in the expr() bound to o:

 int test_fail(const void *p) {
-  return ((const Container*)p)->count();
+  return ((const Container*)p)->size();  // expected
+  return (const Container*)p->size();    // actual
 }

Bug in changeTo() or am I missing something? It looks like any ParenExpr is dropped:

 int test_fail2(const Container &c) {
-  return (c).count();
+  return (c).size();   // expected
+  return c.size();     // actual
 }

Upvotes: 1

Views: 56

Answers (1)

Marc Mutz - mmutz
Marc Mutz - mmutz

Reputation: 25293

It turns out that the on() matcher drops the parentheses:

AST_MATCHER_P(CXXMemberCallExpr, on, internal::Matcher<Expr>,
              InnerMatcher) {
  const Expr *ExprNode = Node.getImplicitObjectArgument()
                            ->IgnoreParenImpCasts();      // <-- HERE
  return (ExprNode != nullptr &&
          InnerMatcher.matches(*ExprNode, Finder, Builder));
}

If I create a new matcher that only ignores implicit casts:

AST_MATCHER_P(CXXMemberCallExpr, onIgnoringImpCasts, Matcher<Expr>,
              InnerMatcher) {
  const Expr *ExprNode = Node.getImplicitObjectArgument()
                            ->IgnoreImpCasts();
  return (ExprNode != nullptr &&
          InnerMatcher.matches(*ExprNode, Finder, Builder));

and use that instead of on(), both test cases pass.

Upvotes: 1

Related Questions