Proposal: add "replace" functionality to clang-query

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Proposal: add "replace" functionality to clang-query

Fangrui Song via cfe-dev
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

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: add "replace" functionality to clang-query

Fangrui Song via cfe-dev
On Tue, Mar 24, 2020 at 4:22 PM Alexander Timin via cfe-dev
<[hidden email]> wrote:

>
> 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


> Alexander
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: add "replace" functionality to clang-query

Fangrui Song via cfe-dev
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

On Wed, Mar 25, 2020 at 12:52 PM Aaron Ballman <[hidden email]> wrote:
On Tue, Mar 24, 2020 at 4:22 PM Alexander Timin via cfe-dev
<[hidden email]> wrote:
>
> 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


> Alexander
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: add "replace" functionality to clang-query

Fangrui Song via cfe-dev
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:

> replace matcher foo = ${foo}
> transform matcher complexStencil(...)


On Wed, 25 Mar 2020 at 17:05, Yitzhak Mandelbaum <[hidden email]> wrote:
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

On Wed, Mar 25, 2020 at 12:52 PM Aaron Ballman <[hidden email]> wrote:
On Tue, Mar 24, 2020 at 4:22 PM Alexander Timin via cfe-dev
<[hidden email]> wrote:
>
> 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


> Alexander
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: add "replace" functionality to clang-query

Fangrui Song via cfe-dev
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.

On Wed, Mar 25, 2020 at 1:47 PM Alexander Timin <[hidden email]> wrote:
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:

> replace matcher foo = ${foo}
> transform matcher complexStencil(...)


On Wed, 25 Mar 2020 at 17:05, Yitzhak Mandelbaum <[hidden email]> wrote:
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

On Wed, Mar 25, 2020 at 12:52 PM Aaron Ballman <[hidden email]> wrote:
On Tue, Mar 24, 2020 at 4:22 PM Alexander Timin via cfe-dev
<[hidden email]> wrote:
>
> 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


> Alexander
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

smime.p7s (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: add "replace" functionality to clang-query

Fangrui Song via cfe-dev
In reply to this post by Fangrui Song via cfe-dev


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:

 https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools

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.


On 25/03/2020 17:47, Alexander Timin via cfe-dev wrote:
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:

> replace matcher foo = ${foo}
> transform matcher complexStencil(...)


On Wed, 25 Mar 2020 at 17:05, Yitzhak Mandelbaum <[hidden email]> wrote:
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

On Wed, Mar 25, 2020 at 12:52 PM Aaron Ballman <[hidden email]> wrote:
On Tue, Mar 24, 2020 at 4:22 PM Alexander Timin via cfe-dev
<[hidden email]> wrote:
>
> 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


> Alexander
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev