Preventing several replacements on a macro call.

[Angel Garcia via cfe-commits]

Hi Argyrios,

Thank you for your answer. I think that it is pretty clear that we don't
really want to allow changes like the one in your example. But my problem
is almost the opposite. I have something like this:

#define MY_MAC(x,y) doSomething(x, y);

which is used in this way:

MY_MAC(var1, var1);

and I want to replace all usages of "var1" to another thing, "var2". Each
of the arguments of the macro is used only once, so it should be safe to
replace each of them independently and obtain

MY_MAC(var2, var2);

But right now what I would obtain is

MY_MAC(var2, );

They look like different cases to me, even though the threshold is not very
clear. Maybe there is a way to allow this transformation, but still
disallow yours. Or do you think that this isn't safe? I started working on
this hardly a month ago, so I am not totally aware of the risks of these
things.

Angel



On Thu, Sep 10, 2015 at 9:01 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com>
wrote:

> Hi Angel,
>
> This part of the code is conservative because it is not clear if accepting
> the change in all the different places where the macro argument is expanded
> is acceptable or not.
> For a contrived example, let’s say you have this macro definition:
>
> #define MY_MAC(x) foo(x) + bar(x)
>
> and you use it like this:
>
> int a = MY_MAC(b);
>
> which expands to this:
>
> int a = foo(b) + bar(b);
>
> Now suppose you want to find all places where “foo(b)” is used and change
> it to “foo(b.c)”. You walk the AST and find “foo(b)” from the macro
> expansion and you change ‘b’ to ‘b.c’. This change will apply to the macro
> argument:
>
> int a = MY_MAC(b.c);
>
> But this now expands like this:
>
> int a = foo(b.c) + bar(b.c)
>
> And you unintentionally also changed the ‘bar’ call.
>
>
> Now, ideally we would keep track of all such changes and if you tried to
> change the ‘b’ in both “foo(b)” and “bar(b)” we would automatically accept
> it; but this is a bit complicated.
> In the meantime we could add a ‘knob' to control this behavior. We could
> have a field in Commit object that you set to true to indicate that it is
> ok to accept a change in a macro argument that expands in multiple places,
> and also for convenience add such a knob to EditedSource object for
> accepting in all commits without needing to set it for each commit
> separately.
>
> What do you think ?
>
> On Sep 9, 2015, at 6:05 AM, Angel Garcia via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> +cfe-commits
>
> On Tue, Sep 8, 2015 at 6:56 PM, Angel Garcia <angelgarcia at google.com>
> wrote:
>
>> Hi Ted,
>>
>> I was working on a clang-tidy check, and today I discovered that it was
>> unable to do several replacements in different arguments of the same macro
>> call. At first, I thought it was a bug, and trying to figure out why this
>> was happening, I found that the replacements were rejected in
>> *lib/Edit/EditedSource.cpp:46*, where there is a comment that says
>> "Trying to write in a macro argument input that has already been written
>> for another argument of the same macro". This comment means that this
>> changes are rejected on purpose.
>>
>> At the commit history, I saw that you had commited
>> <http://reviews.llvm.org/rL152141> this code (that's why I am asking
>> you). Do you think that there is a way around this? I don't really
>> understand why there is a particular case for the macros here, and
>> understanding it would help me to decide whether I should give up on trying
>> to make this work, or try to find a "better" solution.
>>
>> Thanks and sorry for the inconvenience,
>> Angel
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150910/b5235d38/attachment-0001.html>