[cfe-dev] ASTMatcher for assert()

[Manuel Klimek]

On Fri, Jun 6, 2014 at 4:48 PM, Alex Horn <alex.horn at cs.ox.ac.uk> wrote:

> > [...] Why not write all tools to work on the non-macro-rewritten code?
> > (you can get  macros yourself via the PPCallbacks interface).
>
> As far as I understand, PPCallbacks operate on a different level of
> abstraction than ASTMatchers. The RefactoringTool I have in mind
> rewrites two things: (1) "assert()" macros and (2) control-flow
> conditions in "if-then-else", "while" and "for" statements. The
> rewriting of both (1) and (2) is now implemented as ASTMatchers.
> Unfortunately, (2) does not work correctly unless macros are rewritten
> beforehand. To see this, consider "#define compare(a, b) do { if ((a)
> < (b)) swap(a, b) } while(0);".
>

I'm considering it :) So what's the problem rewriting this inside the macro?


> I presumed for now that it does not make sense, generally speaking, to
> invoke ASTMatchers from within PPCallbacks because the source code
> inside a macro definition may not be well-formed. This is why I was
> merely looking for a way to combine ASTMatchers and cc1 drivers.
>

No, my idea on how to use PPCallbacks was to just use them to collect
information about the macros, and then use that information to generate the
replacements.


>
> Best,
> Alex
>
> On 6 June 2014 15:13, Manuel Klimek <klimek at google.com> wrote:
> > On Fri, Jun 6, 2014 at 4:10 PM, Alex Horn <alex.horn at cs.ox.ac.uk> wrote:
> >>
> >> Thanks for the effective response. This helped on the right track and
> >> I've successfully used SourceManager::getImmediateExpansionRange() to
> >> insert BAR and its surrounding parenthesis correctly.
> >>
> >> As a directly related issue: The proposed technique of using the
> >> expansion range does the trick but of course requires that "-cc1
> >> -rewrite-macros" is _not_ invoked prior to rewriting "assert()"
> >> macros. However, this is problematic when other rewrites in the same
> >> RefactoringTool _do_ rely on macros having been rewritten. This poses
> >> the question whether these things can coexist in the same
> >> RefactoringTool. In particular, is it possible to call the
> >> RewriteMacros functionality only after certain matches have completed?
> >>
> >> To ask the same thing differently, what is the preferred way to
> >> compose ASTMatchers and cc1 drivers such as RewriteMacros?
> >
> >
> > I wouldn't combine them. Why not write all tools to work on the
> > non-macro-rewritten code? (you can get macros yourself via the
> PPCallbacks
> > interface).
> >
> >>
> >>
> >> On 6 June 2014 13:56, Manuel Klimek <klimek at google.com> wrote:
> >> > On Tue, Jun 3, 2014 at 5:36 PM, Alex Horn <alex.horn at cs.ox.ac.uk>
> wrote:
> >> >>
> >> >> Hello,
> >> >>
> >> >> I am struggling to replace "assert(c)" macro calls by "BAR(c)" where
> c
> >> >> is some integer expression and BAR is a function.
> >> >>
> >> >> The problem manifests itself in RefactoringTool::applyAllReplacements
> >> >> which returns false, thereby causing the tool to skip replacements.
> >> >>
> >> >> I am using the following ASTMatcher:
> >> >>
> >> >> StatementMatcher makeAssertMatcher() {
> >> >>   return
> >> >>
> >> >>
> callExpr(callee(functionDecl(hasName("__builtin_expect")))).bind("AssertBindId");
> >> >> }
> >> >>
> >> >> Note: callExpr(callee(functionDecl(hasName("assert"))) won't work
> here
> >> >> because assert() is a macro according to the assert.h and cassert
> >> >> headers.
> >> >>
> >> >> Here's an example match using clang-query:
> >> >>
> >> >> example.cpp:
> >> >>
> >> >>   #include <assert.h>
> >> >>   int main() {
> >> >>     assert(false);
> >> >>     return 0;
> >> >>   }
> >> >>
> >> >> $ clang-query> match
> >> >> callExpr(callee(functionDecl(hasName("__builtin_expect"))))
> >> >>
> >> >>   Match #1:
> >> >>
> >> >>   example.cpp:4:3: note: "AssertBindId" binds here
> >> >>     assert(false);
> >> >>     ^~~~~~~~~~~~~
> >> >>   /usr/include/assert.h:93:6: note: expanded from macro 'assert'
> >> >>       (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__,
> >> >> __LINE__, #e) : (void)0)
> >> >>        ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> >>   1 match.
> >> >>
> >> >> But I cannot find the correct way to rewrite the match; the following
> >> >> replacement is skipped (i.e. see line 295 in Refactoring.cpp):
> >> >>
> >> >>   const CallExpr *E =
> Result.Nodes.getNodeAs<CallExpr>("AssertBindId");
> >> >>   SourceManager &SM = *Result.SourceManager;
> >> >>   SourceLocation LocBegin = E->getLocStart();
> >> >>   Replace->insert(tooling::Replacement(SM, LocBegin,
> >> >>     /* number of letters in assert */ 6, "BAR"));
> >> >>
> >> >> What is the correct way (if any) to rewrite call expressions of
> macros
> >> >> such as "assert()"?
> >> >
> >> >
> >> > You need to get the expansion location:
> >> > SourceLocation ExpLocBegin = SM.getExpansionLoc(LocBegin);
> >> > Replace->insert(tooling::Replacement(SM, ExpLocBegin, 6, "BAR"));
> >> >
> >> > Something like this should do the trick...
> >> > Cheers,
> >> > /Manuel
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140606/9dd27666/attachment.html>