Rewriting function calls

Hi there,

I am thinking about reverse the order of arguments in a call expression with clang. I'm leveraging the refactoring tool and the AST matcher framework. I use a matcher to find those calls i'm interested in, rewrite the expr, and insert one replacement. Things go tricky for nested calls. Because the refactoring tool applies replacements one by one, and order adjustment does not change the total text length, one replacement could overwrite prior changes. Do i have to implement my own replacement applying algo (i'm thinking about adding source range references to a replacement object, and applying them recursively), or is there a simple and better way to do this?

Thanks!

Hi there,

I am thinking about reverse the order of arguments in a call expression
with clang. I'm leveraging the refactoring tool and the AST matcher
framework. I use a matcher to find those calls i'm interested in, rewrite
the expr, and insert one replacement. Things go tricky for nested calls.
Because the refactoring tool applies replacements one by one, and order
adjustment does not change the total text length, one replacement could
overwrite prior changes. Do i have to implement my own replacement applying
algo (i'm thinking about adding source range references to a replacement
object, and applying them recursively), or is there a simple and better way
to do this?

The problem is that I think the problem is not solvable in general - there
is a class of replacements that's recursively applicable though. I think it
makes sense to add support for those in the refactoring library. I have no
clue yet how to do that though :slight_smile:

The easy workaround is to do one non-overlapping refactoring at a time
(that has worked every time so far for us)[

Cheers,
/Manuel

Can you give an example of what you mean by a recursively applicable replacement?

Imagine you have a 2-arg join(a, join(b, join(c, d)))

Now you get C++11 so you implement a fancy n-arg one, and want to simplify all your code. Thus, you’ll want
join(a, b, c, d) as the result.
Now this would work as a source location mapping preserving transformation of the following steps:

  1. join(a, join(b, c, d))

  2. join(a, b, c, d)

In general, the Rewriter is able to keep mappings through rewrites, but I don’t know how far this would work on this example…

Cheers,
/Manuel

ÔÚ 2012-12-7£¬15:15£¬Manuel Klimek <klimek@google.com> дµÀ£º

Hi there,

I am thinking about reverse the order of arguments in a call expression with clang. I'm leveraging the refactoring tool and the AST matcher framework. I use a matcher to find those calls i'm interested in, rewrite the expr, and insert one replacement. Things go tricky for nested calls. Because the refactoring tool applies replacements one by one, and order adjustment does not change the total text length, one replacement could overwrite prior changes. Do i have to implement my own replacement applying algo (i'm thinking about adding source range references to a replacement object, and applying them recursively), or is there a simple and better way to do this?

The problem is that I think the problem is not solvable in general - there is a class of replacements that's recursively applicable though. I think it makes sense to add support for those in the refactoring library. I have no clue yet how to do that though :slight_smile:

The easy workaround is to do one non-overlapping refactoring at a time (that has worked every time so far for us)[

Can you please share a little bit more details about your workaround? For this particular nested call case, did you mean running the refactoring tool for several passes? If so, I may not be able to adopt this approach because in my case a partially replaced function call cannot compile, which means the AST may not be well built.

Thanks!

If you tell me more about your specific case, I might be able to come up
with ideas that apply to it directly :slight_smile:

Cheers,
/Manuel

Hi,

Basically, I'd like to replace a class Bar with another class Baz. Most of their interfaces are identical except:

* Some functions have incompatible order of parameters. So I need to re-arrange the order of argument for those Bar member functions calls.
* Some functions in Bar don't have their counterparts in Baz. In this case, the original calls have to be rewritten. Usually they are transformed into calls to helper functions.

I'm considering configuring the transformation with a simple template language. For example, this rule

  Bar::foo(size_t, bool) => foo($2, $1)

means reversing the 2 arguments.

Supposing Bar::foo(size_t, bool) returns a bool, rewriting a call like

  bar.foo(1, bar.foo(2, false))

in two passes seems not working. Say first we refactor the code as

  bar.foo(1, bar.foo(false, 2))

Now the code obviously won't compile, no matter if we have rewritten the type of bar. I think it might be handy to have a replacement class that can reference source ranges in a rewrite rope.

ÔÚ 2012-12-8£¬00:52£¬Manuel Klimek <klimek@google.com> дµÀ£º

Hi,

Basically, I'd like to replace a class Bar with another class Baz. Most of
their interfaces are identical except:

* Some functions have incompatible order of parameters. So I need to
re-arrange the order of argument for those Bar member functions calls.
* Some functions in Bar don't have their counterparts in Baz. In this
case, the original calls have to be rewritten. Usually they are transformed
into calls to helper functions.

I'm considering configuring the transformation with a simple template
language. For example, this rule

  Bar::foo(size_t, bool) => foo($2, $1)

means reversing the 2 arguments.

Supposing Bar::foo(size_t, bool) returns a bool, rewriting a call like

  bar.foo(1, bar.foo(2, false))

in two passes seems not working. Say first we refactor the code as

  bar.foo(1, bar.foo(false, 2))

Now the code obviously won't compile, no matter if we have rewritten the
type of bar. I think it might be handy to have a replacement class that can
reference source ranges in a rewrite rope.

If I'm not mistaken that's exactly the way the Rewriter handles changes (in
a rewrite rope). I haven't extensively tested this, but I'd be interested
in what the limitations are (most of our internal rewrites need multi-step
processed anyway, due to code review constraints etc)

That said, you can easily special-case stuff up to a certain level - that
will not get you the 100% case, but it's usually enough to get the 99% case
even on very large code bases.

Cheers,
/Manuel

Hi Manuel,

Yes I know that Rewriter manages changes in a rope. Let me state it in another way. IIUC, tooling::Replacement encapsulates a file offset, length of the original text, and the replacement text. The strawman's approach to rewriting Bar::foo(size_t, bool) is to use a memberCallExpr() matcher to find all their occurrences. For each of them, we use Lexer to extract the source text and insert a replacement. This works for situations without nested calls. Now for code

  bar.foo(1, bar.foo(2, false))

We first match the outer call and add a replacement that would change the source to

  bar.foo(bar.foo(2, false), 1)

and then the inner call match adds a second replacement, which ideally would change bar.foo(2, false) to bar.foo(false, 2).

Since RefactoringTool applies replacements sequentially, the second replacement would be applied at the wrong position.

So my point (although just a wild thought) was that it might be helpful to have a replacement that can reference a range in the rewriter buffer rope. To actually apply a replacement x, we need to first dereference it to get the actual text. If that range is to be modified in another replacement object y, that y has to be applied before x. Perhaps this is the recursively applicable changes that you mentioned earlier.

Thoughts? Or did I miss something?

ÔÚ 2012-12-8£¬01:15£¬Manuel Klimek <klimek@google.com> дµÀ£º