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

[Stephen Kelly]

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?

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 :).

>> [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.

> 
> 
>>
>> 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. 

Thanks,

Steve.