<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 10, 2015, at 12:22 PM, Angel Garcia via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hi Argyrios,<div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">#define MY_MAC(x,y) doSomething(x, y);</div><div class=""><br class=""></div><div class="">which is used in this way:</div><div class=""><br class=""></div><div class="">MY_MAC(var1, var1);</div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">MY_MAC(var2, var2);</div><div class=""><br class=""></div><div class="">But right now what I would obtain is</div><div class=""><br class=""></div><div class="">MY_MAC(var2, );</div></div></div></blockquote><div><br class=""></div><div>That doesn’t seem like a case where the change was rejected but where it was misapplied.</div><div>Could you provide a reduced test case along with your clang-tidy changes and how to invoke it against the test case ?</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">Angel</div><div class=""><br class=""></div><div class=""><br class=""></div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Sep 10, 2015 at 9:01 PM, Argyrios Kyrtzidis <span dir="ltr" class=""><<a href="mailto:kyrtzidis@apple.com" target="_blank" class="">kyrtzidis@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">Hi Angel,</div><div class=""><br class=""></div><div class="">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 class="">For a contrived example, let’s say you have this macro definition:</div><div class=""><br class=""></div><div class=""><span style="white-space:pre-wrap" class=""> </span>#define MY_MAC(x) foo(x) + bar(x)</div><div class=""><br class=""></div><div class="">and you use it like this:</div><div class=""><br class=""></div><div class=""><span style="white-space:pre-wrap" class=""> </span>int a = MY_MAC(b);</div><div class=""><br class=""></div><div class="">which expands to this:</div><div class=""><br class=""></div><div class=""><span style="white-space:pre-wrap" class=""> </span>int a = foo(b) + bar(b);</div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class=""><span style="white-space:pre-wrap" class=""> </span>int a = MY_MAC(b.c);</div><div class=""><br class=""></div><div class="">But this now expands like this:</div><div class=""><br class=""></div><div class=""><span style="white-space:pre-wrap" class=""> </span>int a = foo(b.c) + bar(b.c)</div><div class=""><br class=""></div><div class="">And you unintentionally also changed the ‘bar’ call.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">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 class="">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 class=""><br class=""></div><div class="">What do you think ?</div><br class=""><div class=""><blockquote type="cite" class=""><div class=""><div class="h5"><div class="">On Sep 9, 2015, at 6:05 AM, Angel Garcia via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>> wrote:</div><br class=""></div></div><div class=""><div class=""><div class="h5"><div dir="ltr" class="">+cfe-commits</div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Sep 8, 2015 at 6:56 PM, Angel Garcia <span dir="ltr" class=""><<a href="mailto:angelgarcia@google.com" target="_blank" class="">angelgarcia@google.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="">Hi Ted,<div class=""><br class=""></div><div class="">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 class="">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 class=""><br class=""></div><div class="">At the commit history, I saw that you had <a href="http://reviews.llvm.org/rL152141" target="_blank" class="">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 class=""><br class=""></div><div class="">Thanks and sorry for the inconvenience,</div><div class="">Angel</div></div>
</blockquote></div><br class=""></div></div></div>
_______________________________________________<br class="">cfe-commits mailing list<br class=""><a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class=""></div></blockquote></div><br class=""></div></blockquote></div><br class=""></div>
_______________________________________________<br class="">cfe-commits mailing list<br class=""><a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits<br class=""></div></blockquote></div><br class=""></body></html>