[cfe-dev] Qt 4 to Qt 5 porting tool with the tooling branch

[Manuel Klimek]

On Mon, Jun 18, 2012 at 4:13 PM, Stephen Kelly <steveire at gmail.com> wrote:

> Manuel Klimek wrote:
>
> > Hi Stephen,
> >
> > first, really cool blog post, and thanks for providing feedback & docs
> and
> > helping us making the tools more awesome - we really appreciate this!
>
> No problem. Clang based porting tools is a cool party :).
>
> >> After that, the matches are actually nested function calls (the
> >> capitalisation style for functions seems funny to use Qt developers).
> >> Nested
> >>
> >
> > Yea, that's not set yet - please chime in with your color of the bikeshed
> > on the review thread for the ast matchers ;) I seriously don't care, so I
> > want people who care about one way or the other to jump in with good
> > arguments of why their preferred way is superior :D
>
> Is the review thread on cfe-commits? I'm not really familiar with how
> reviews are done in this community yet. Does this mean that the content of
> the branch is being rebased and applied to trunk?
>

Yep. The content of the branch is "rebased" (well, as far as you can do
that in svn) continuously anyway (usually about once per week, or when
somebody screams at me because the compile breaks).
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120611/059010.html



> I don't think I'll jump in on this bikeshed aspect though :). The case is
> something I noted for colleagues who use Qt all day, where the convention
> is
> methods/functions = lowercase first letter. If you want to be 'compatible'
> with Qt, they would need to be changed, but otherwise it's not important
> :).
>

Fair enough...


>
> >> [End copy-paste]
> >>
> >
> > All accurate.
>
> Great, thanks.
>
> >
> >> I was also wondering if there is an easy way to only modify the code
> >> inside the repo and not included headers? In my tool I require the user
> >> to pass the
> >> source-dir on the command line, and then manually check paths to see if
> >> they
> >> are below it, but this is error-prone.
> >>
> >
> > If with "manually", you mean "do a regexp match on the path", then yep,
> > that's what I also do.
> > Why is that error prone?
>
> There could be aspects of relative/absolute paths, symlinks, os-specific
> issues, bugs in the regexp, etc.
>
> >> Additionally, many of the concepts I encoded are generic, not Qt
> specific
> >> (such as renaming methods, enums etc). It seems that these could be re-
> >> usable, so we could either add some version of this to the clang
> >> examples, and/or see if we can define a higher level collection of ways
> >> to refactor the code. I presume google also has some collection/repo of
> >> such things already?
> >>
> >
> > Unfortunately not. Most of the engineers using those tools at Google have
> > pretty non-standard and complex problems, at which point other costs
> start
> > to dwarf writing the matchers and replacements.
> >
> > But we are starting to work on a more principled approach that includes
> > ready-made refactorings, targeted towards small scale use cases ("I want
> > to rename what is under my cursor", we actually have a prototype for this
> > in the branch).
>
> The ClangRename example?
>
> Yes, that's also useful in the context of an IDE I think, but in terms of
> our goals of a 'fire and forget' tool that does the boring parts of a
> porting job, it's not really so important to rename things by symbol under
> cursor or at a location in a file.
>

Good point.


> >
> >
> >>
> >> Finally, the suggestion here:
> >>
> >>
> http://thread.gmane.org/gmane.comp.compilers.clang.devel/20433/focus=20759
> >>
> >> was not quite what I needed to use to correctly match the code used. The
> >> matcher I used was this:
> >>
> >>  Finder.addMatcher(
> >>    Id("call",
> >>      Call(
> >>        Callee(Function(HasName(QtEscapeFunction))),
> >>        HasArgument(
> >>          0,
> >>          AnyOf(
> >>            BindTemporaryExpression(has(Id("ctor", ConstructorCall()))),
> >>            BindTemporaryExpression(has(Id("operator",
> >> OverloadedOperatorCall()))),
> >>            Id("operator", OverloadedOperatorCall()),
> >>            Id("ctor", ConstructorCall()),
> >>            Id("expr", Expression())
> >>          )
> >>        )
> >>      )
> >>    ),
> >>    &Callback);
> >>
> >> So I had to add some extra BindTemporaryExpression() into the expression
> >> which I would have preferred to be automatic, like I think some implicit
> >> cast stuff is.
> >>
> >
> > Yep, the temporary stuff is not yet perfect. Ideas welcome, preferably
> > when we have it in mainline ;) (there's a review thread currently open -
> > afaik nobody answered yet).
>
> I'd like to have a look if you can link it.
>
>
See link above. Feedback welcome!

Cheers,
/Manuel


> Thanks,
>
> Steve.
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120618/bced5495/attachment.html>