<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 6, 2014 at 6:04 PM, Alex Horn <span dir="ltr"><<a href="mailto:alex.horn@cs.ox.ac.uk" target="_blank">alex.horn@cs.ox.ac.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> [W]hat's the problem rewriting this inside the macro?<br>
<br>
The problem I encountered was that Clang would not automatically<br>
invoke the ASTMatchers inside the macro definitions. The comments I<br>
found before I posted on this mailing list indicated that this is by<br>
design and that it can only be circumvented by rewriting macros<br>
beforehand, i.e. -rewrite-macros. But your previous remark gives me<br>
hope that the situation might have changed, or that I have just missed<br>
the right mailing list comments.<br></blockquote><div><br></div><div>Well, the macro has to be actually used ;) Even if it's a library, I'd hope you have at least a test that uses the macro... (same for templates btw).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> [Use PPCallbacks to] collect information about the macros, and then use that information to generate the replacements.<br>
<br>
I'm afraid that with my limited expertise in Clang I would have to<br>
look at an example that illustrates the technique you've in mind. It<br>
may be even that I have not articulated clearly enough and that we are<br>
thinking of two different things here.<br></blockquote><div><br></div><div>I'm, not sure you need that kind of information. See above. Code that is in macros will be visited at every expansion location.</div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On 6 June 2014 16:24, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
> On Fri, Jun 6, 2014 at 4:48 PM, Alex Horn <<a href="mailto:alex.horn@cs.ox.ac.uk">alex.horn@cs.ox.ac.uk</a>> wrote:<br>
>><br>
>> > [...] Why not write all tools to work on the non-macro-rewritten code?<br>
>> > (you can get macros yourself via the PPCallbacks interface).<br>
>><br>
>> As far as I understand, PPCallbacks operate on a different level of<br>
>> abstraction than ASTMatchers. The RefactoringTool I have in mind<br>
>> rewrites two things: (1) "assert()" macros and (2) control-flow<br>
>> conditions in "if-then-else", "while" and "for" statements. The<br>
>> rewriting of both (1) and (2) is now implemented as ASTMatchers.<br>
>> Unfortunately, (2) does not work correctly unless macros are rewritten<br>
>> beforehand. To see this, consider "#define compare(a, b) do { if ((a)<br>
>> < (b)) swap(a, b) } while(0);".<br>
><br>
><br>
> I'm considering it :) So what's the problem rewriting this inside the macro?<br>
><br>
>><br>
>> I presumed for now that it does not make sense, generally speaking, to<br>
>> invoke ASTMatchers from within PPCallbacks because the source code<br>
>> inside a macro definition may not be well-formed. This is why I was<br>
>> merely looking for a way to combine ASTMatchers and cc1 drivers.<br>
><br>
><br>
> No, my idea on how to use PPCallbacks was to just use them to collect<br>
> information about the macros, and then use that information to generate the<br>
> replacements.<br>
><br>
>><br>
>><br>
>> Best,<br>
>> Alex<br>
>><br>
>> On 6 June 2014 15:13, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>> > On Fri, Jun 6, 2014 at 4:10 PM, Alex Horn <<a href="mailto:alex.horn@cs.ox.ac.uk">alex.horn@cs.ox.ac.uk</a>> wrote:<br>
>> >><br>
>> >> Thanks for the effective response. This helped on the right track and<br>
>> >> I've successfully used SourceManager::getImmediateExpansionRange() to<br>
>> >> insert BAR and its surrounding parenthesis correctly.<br>
>> >><br>
>> >> As a directly related issue: The proposed technique of using the<br>
>> >> expansion range does the trick but of course requires that "-cc1<br>
>> >> -rewrite-macros" is _not_ invoked prior to rewriting "assert()"<br>
>> >> macros. However, this is problematic when other rewrites in the same<br>
>> >> RefactoringTool _do_ rely on macros having been rewritten. This poses<br>
>> >> the question whether these things can coexist in the same<br>
>> >> RefactoringTool. In particular, is it possible to call the<br>
>> >> RewriteMacros functionality only after certain matches have completed?<br>
>> >><br>
>> >> To ask the same thing differently, what is the preferred way to<br>
>> >> compose ASTMatchers and cc1 drivers such as RewriteMacros?<br>
>> ><br>
>> ><br>
>> > I wouldn't combine them. Why not write all tools to work on the<br>
>> > non-macro-rewritten code? (you can get macros yourself via the<br>
>> > PPCallbacks<br>
>> > interface).<br>
>> ><br>
>> >><br>
>> >><br>
>> >> On 6 June 2014 13:56, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>> >> > On Tue, Jun 3, 2014 at 5:36 PM, Alex Horn <<a href="mailto:alex.horn@cs.ox.ac.uk">alex.horn@cs.ox.ac.uk</a>><br>
>> >> > wrote:<br>
>> >> >><br>
>> >> >> Hello,<br>
>> >> >><br>
>> >> >> I am struggling to replace "assert(c)" macro calls by "BAR(c)" where<br>
>> >> >> c<br>
>> >> >> is some integer expression and BAR is a function.<br>
>> >> >><br>
>> >> >> The problem manifests itself in<br>
>> >> >> RefactoringTool::applyAllReplacements<br>
>> >> >> which returns false, thereby causing the tool to skip replacements.<br>
>> >> >><br>
>> >> >> I am using the following ASTMatcher:<br>
>> >> >><br>
>> >> >> StatementMatcher makeAssertMatcher() {<br>
>> >> >> return<br>
>> >> >><br>
>> >> >><br>
>> >> >> callExpr(callee(functionDecl(hasName("__builtin_expect")))).bind("AssertBindId");<br>
>> >> >> }<br>
>> >> >><br>
>> >> >> Note: callExpr(callee(functionDecl(hasName("assert"))) won't work<br>
>> >> >> here<br>
>> >> >> because assert() is a macro according to the assert.h and cassert<br>
>> >> >> headers.<br>
>> >> >><br>
>> >> >> Here's an example match using clang-query:<br>
>> >> >><br>
>> >> >> example.cpp:<br>
>> >> >><br>
>> >> >> #include <assert.h><br>
>> >> >> int main() {<br>
>> >> >> assert(false);<br>
>> >> >> return 0;<br>
>> >> >> }<br>
>> >> >><br>
>> >> >> $ clang-query> match<br>
>> >> >> callExpr(callee(functionDecl(hasName("__builtin_expect"))))<br>
>> >> >><br>
>> >> >> Match #1:<br>
>> >> >><br>
>> >> >> example.cpp:4:3: note: "AssertBindId" binds here<br>
>> >> >> assert(false);<br>
>> >> >> ^~~~~~~~~~~~~<br>
>> >> >> /usr/include/assert.h:93:6: note: expanded from macro 'assert'<br>
>> >> >> (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__,<br>
>> >> >> __LINE__, #e) : (void)0)<br>
>> >> >> ^~~~~~~~~~~~~~~~~~~~~~~~~<br>
>> >> >> 1 match.<br>
>> >> >><br>
>> >> >> But I cannot find the correct way to rewrite the match; the<br>
>> >> >> following<br>
>> >> >> replacement is skipped (i.e. see line 295 in Refactoring.cpp):<br>
>> >> >><br>
>> >> >> const CallExpr *E =<br>
>> >> >> Result.Nodes.getNodeAs<CallExpr>("AssertBindId");<br>
>> >> >> SourceManager &SM = *Result.SourceManager;<br>
>> >> >> SourceLocation LocBegin = E->getLocStart();<br>
>> >> >> Replace->insert(tooling::Replacement(SM, LocBegin,<br>
>> >> >> /* number of letters in assert */ 6, "BAR"));<br>
>> >> >><br>
>> >> >> What is the correct way (if any) to rewrite call expressions of<br>
>> >> >> macros<br>
>> >> >> such as "assert()"?<br>
>> >> ><br>
>> >> ><br>
>> >> > You need to get the expansion location:<br>
>> >> > SourceLocation ExpLocBegin = SM.getExpansionLoc(LocBegin);<br>
>> >> > Replace->insert(tooling::Replacement(SM, ExpLocBegin, 6, "BAR"));<br>
>> >> ><br>
>> >> > Something like this should do the trick...<br>
>> >> > Cheers,<br>
>> >> > /Manuel<br>
>> ><br>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>