<div dir="ltr">Hi Argyrios,<div><br></div><div>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:<br><br></div><div>#define MY_MAC(x,y) doSomething(x, y);</div><div><br></div><div>which is used in this way:</div><div><br></div><div>MY_MAC(var1, var1);</div><div><br></div><div>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</div><div><br></div><div>MY_MAC(var2, var2);</div><div><br></div><div>But right now what I would obtain is</div><div><br></div><div>MY_MAC(var2, );</div><div><br></div><div>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.</div><div><br></div><div>Angel</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 10, 2015 at 9:01 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:kyrtzidis@apple.com" target="_blank">kyrtzidis@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Hi Angel,</div><div><br></div><div>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.</div><div>For a contrived example, let’s say you have this macro definition:</div><div><br></div><div><span style="white-space:pre-wrap"> </span>#define MY_MAC(x) foo(x) + bar(x)</div><div><br></div><div>and you use it like this:</div><div><br></div><div><span style="white-space:pre-wrap"> </span>int a = MY_MAC(b);</div><div><br></div><div>which expands to this:</div><div><br></div><div><span style="white-space:pre-wrap"> </span>int a = foo(b) + bar(b);</div><div><br></div><div>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:</div><div><br></div><div><span style="white-space:pre-wrap"> </span>int a = MY_MAC(b.c);</div><div><br></div><div>But this now expands like this:</div><div><br></div><div><span style="white-space:pre-wrap"> </span>int a = foo(b.c) + bar(b.c)</div><div><br></div><div>And you unintentionally also changed the ‘bar’ call.</div><div><br></div><div><br></div><div>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.</div><div>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.</div><div><br></div><div>What do you think ?</div><br><div><blockquote type="cite"><div><div class="h5"><div>On Sep 9, 2015, at 6:05 AM, Angel Garcia via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:</div><br></div></div><div><div><div class="h5"><div dir="ltr">+cfe-commits</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 8, 2015 at 6:56 PM, Angel Garcia <span dir="ltr"><<a href="mailto:angelgarcia@google.com" target="_blank">angelgarcia@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Ted,<div><br></div><div>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 <i>lib/Edit/EditedSource.cpp:46</i>, 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.</div><div><br></div><div>At the commit history, I saw that you had <a href="http://reviews.llvm.org/rL152141" target="_blank">commited</a> 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.</div><div><br></div><div>Thanks and sorry for the inconvenience,</div><div>Angel</div></div>
</blockquote></div><br></div></div></div>
_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br></div></blockquote></div><br></div></blockquote></div><br></div>