Proposal: add "replace" functionality to clang-query

Hey!

I think that medium-scale medium-complexity C++ refactorings (like replacing the order of the two arguments of a function across a large codebase) are much harder that they should be: clang-refactor and clang-rename do not cover them and writing a full libTooling tool is not the most time-efficient solution.

I propose extending the functionality of clang-query to cover this use case and allow generating replacements (which can be then applied by clang-apply-replacements) from matches:

replace MATCHER NODE PATTERN

for example,

let f = cxxMethodDecl(hasName(“Foo”))
let arg1 = hasArgument(0, expr().bind(“arg1”))
let arg2 = hasArgument(1, expr().bind(“arg2”))

let m = callExpr(on(f), arg1, arg2)
replace m root Foo(${arg2}, ${arg1})

A more detailed design doc is available here.

An early prototype patch is available here.
A real-life example: the query is here, the output in the form of a Chromium patch is here.

WDYT?
Alexander

Hey!

I think that medium-scale medium-complexity C++ refactorings (like replacing the order of the two arguments of a function across a large codebase) are much harder that they should be: clang-refactor and clang-rename do not cover them and writing a full libTooling tool is not the most time-efficient solution.

I propose extending the functionality of clang-query to cover this use case and allow generating replacements (which can be then applied by clang-apply-replacements) from matches:

replace MATCHER NODE PATTERN

for example,

let f = cxxMethodDecl(hasName("Foo"))
let arg1 = hasArgument(0, expr().bind("arg1"))
let arg2 = hasArgument(1, expr().bind("arg2"))
let m = callExpr(on(f), arg1, arg2)
replace m root Foo(${arg2}, ${arg1})

A more detailed design doc is available here.

An early prototype patch is available here.
A real-life example: the query is here, the output in the form of a Chromium patch is here.

WDYT?

Thank you for bringing up this idea, I've wanted something that can do
this for a while. I think the idea has a lot of merit, but I'm
wondering whether we want to focus on code transformations within
clang-query like this as opposed to putting that effort into the
in-progress Transformer work
(http://lists.llvm.org/pipermail/cfe-dev/2019-January/060950.html).
Are you aware of those efforts? If so, how do you see this proposal
coexisting with that functionality?

~Aaron

Aaron, thanks for mentioning Clang Transformer. Alexander, you can find the original doc here. The code is under Tooling: https://github.com/llvm/llvm-project/tree/master/clang/include/clang/Tooling/Transformer

Thanks Yitzhak and Aaron!

As an outsider to the clang land, I wasn’t aware of the Transformer — thanks for pointing me to it, it looks really exciting.

I think that my proposal can be broken down into two parts:
a) Adding an option to generate replacements without having to build a C++ tool.
b) Particular syntax to generate these replacements.

I think that Transformer effort covers only the b) part — please correct me if I’m wrong.

And out of these two, I (looking at the things from the user perspective) think that a) is the most important one, as having to write
and link C++ code adds a lot of friction to the process (so sed + manual edits are still faster). So I think that there is a nice synergy
potential in the long term — I think that the ideal end state would be having the ability to write Transformer-based queries dynamically in
clang-query, something like that:

$ clang-query

transform matcher makeStencil(ifBound(“value”, true, false))

However (as building the dynamic Stencils sounds non-trivial), I think that there is no particular harm in having both options available in clang-query, one being easier to write and one being more flexible:

Agreed on all points. Indeed, we have built a lightweight syntax for ourselves but have not had the opportunity to upstream it yet. If you’re interested, I’d be happy to work with you on that.

Yes, Transformer seems to be what you’re looking for. I made a proof-of-concept GUI tool which integrates Transformer which you can see here:

I think a GUI integration makes more sense than clang-query, so you might find that pursuing a GUI approach would make it easier to expose Transformer features.

Thanks,

Stephen.