> On Sep 10, 2015, at 12:22 PM, Angel Garcia via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > 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, );
That doesn’t seem like a case where the change was rejected but where it was misapplied. Could you provide a reduced test case along with your clang-tidy changes and how to invoke it against the test case ? > > 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 <kyrtzi...@apple.com > <mailto:kyrtzi...@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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> >> +cfe-commits >> >> On Tue, Sep 8, 2015 at 6:56 PM, Angel Garcia <angelgar...@google.com >> <mailto:angelgar...@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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
