Hi Manuel,<div><br></div><div>my rewrite basing only in source replacements is complete, but now I'm facing some strange side effects on some files. Consider the example:</div><div><br></div><div>header ----------</div>
<div><br></div><div>#include <B.h> // B defines a simple class named B</div><div><br></div><div>class A {</div><div>public:</div><div> A(); </div><div><br></div><div> float x;</div><div> B b;</div><div>};</div><div>
<br></div><div>source ---------------</div><div><br></div><div>A::A() : x(0) {</div><div>}</div><div><br></div><div><br></div><div>the strange behavior that I'm having is that if in the header file "x" is declared before "b", when processing the source location for the initializer list on CXXConstructorDecl*, the source location for it get's completely wrong pointing to right before the constructor name: "A::". If i declare "b" before "x" then the source location points to "A::A() : " which was expected. Also, the method getNumCtorInitializers on this declaration is returning to me 2 ( should be 1 as only x is being initialized ), but if decide to initialize b I'm __still__ getting 2. Also, node that the class I'm parsing does not have a base class, and I'm providing the correct includes as the tool run fine and exits returning 0. I'm a bit lost here, any comments are welcome. I could upload on gist the current tool source, cmake and sample code I'm using to test it if someone is willing to try. </div>
<div><br></div><div>I've also tried to dump the ast and verify it but the initalizer list is not showing up so I don't know what might be happening.</div><div><br></div><div><br></div><div>Cheers, </div><div><br></div>
<div>Victor</div><div><br><div class="gmail_quote">2012/8/17 Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Sat, Aug 18, 2012 at 12:16 AM, Victor Vicente de Carvalho<br>
<<a href="mailto:victor.v.carvalho@gmail.com">victor.v.carvalho@gmail.com</a>> wrote:<br>
> 2012/8/17 Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>><br>
>> On Fri, Aug 17, 2012 at 9:34 PM, Victor Vicente de Carvalho<br>
>> <<a href="mailto:victor.v.carvalho@gmail.com">victor.v.carvalho@gmail.com</a>> wrote:<br>
>> > Hello Manuel,<br>
>> ><br>
>> > Sadly, no. I'm struggling with some problems now:<br>
>> ><br>
>> > 1 - How do I detect and generate the class name for nested classes? I'm<br>
>> > using CXXConstructorDecl getNameAsString() but it only returns to me<br>
>> > the<br>
>> > name of the enclosing class.<br>
>><br>
>> As Jends said, getQualifiedNameAsString.<br>
>><br>
><br>
> Thanks! It worked<br>
><br>
><br>
>><br>
>> > 2 - I would like to not replace constructor initializers that were set<br>
>> > up<br>
>> > with macros with their values:<br>
>> ><br>
>> > #define MACRO 10<br>
>> ><br>
>> > Class::Class() : myval(MACRO)<br>
>> ><br>
>> > after I rewrite the outputted code is:<br>
>> ><br>
>> > Class::Class() : mvyal(10)<br>
>> ><br>
>> > Is there a way to avoid this? I've tried to use<br>
>> > Compiler.getPreprocessor().SetMacroExpansionOnlyInDirectives() but this<br>
>> > simply removes the myval definition from the generated AST<br>
>><br>
>> Source locations have the notion of an expansion and a spelling<br>
>> location. To get the code that was originally written, you'll need to<br>
>> fiddle around with those. In your case, MACRO is the expansion<br>
>> location of the literal 10.<br>
>><br>
>> > 3 - Variables that are default initialized on the header are stored with<br>
>> > it.<br>
>> > This is a problem as the compiler complains when I rewrite the source<br>
>> > file<br>
>> > with the initialization on the source but it was previously defined on<br>
>> > the<br>
>> > header. Is there to detect if the default initializer is on the method<br>
>> > definition or in the method declaration?<br>
>><br>
>> I would assume so - I don't where to look of the top of my head though.<br>
>><br>
>> ><br>
>> > 4 - I've found a bug on the DeclPrinter class, and fixed it. The<br>
>> > attachment<br>
>> > is the patch.<br>
>><br>
>> I suggest you send an email to cfe-commits with the subject [PATCH]<br>
>> Fix problem with DeclPrinter, containing the patch and what was wrong<br>
>> (and ideally an automated regression test ;)<br>
>><br>
>> ><br>
>> > 5 - The current way i'm using to replace the old constructor declaration<br>
>> > with the new declaration is with a new range:<br>
>> ><br>
>> > SourceRange rng(d->getSourceRange().getBegin(),<br>
>> > d->getBody()->getSourceRange().getBegin().getLocWithOffset(-1));<br>
>> ><br>
>> > This was the only way I've managed to get the correct end of the end of<br>
>> > the<br>
>> > initializer list. This is an inconvenience as I would like to not<br>
>> > process<br>
>> > the body and hence make the tool faster, but if I disable body parsing I<br>
>> > can't get its SourceRange.<br>
>><br>
>> As far as I understand (and I might well be wrong there) skipping body<br>
>> parsing has some very concrete use cases, and source-to-source<br>
>> transformations is not on of them...<br>
>><br>
>> Cheers,<br>
>> /Manuel<br>
>><br>
><br>
> Manuel, I think I've been thinking the wrong way. As the only thing I need<br>
> is to reorder the initialization, I do not need to mess with the original<br>
> source like I was doing and just concatenate the reordered, original<br>
> CXXCtorInitializer string data. So I've been wondering how I'm supposed to<br>
> recover the source string based on a SourceLocation. Should I use<br>
> SourceManager's getCharacterData? There is a way to recover the entire<br>
> string based on a sourceRange?<br>
<br>
</div></div>RefactoringTool in lib/Tooling has some examples of how to deal with<br>
that. They're not perfect yet, as source locations can be a little<br>
fiddly to handle. In general, SourceManager's getCharacterData sounds<br>
like a good way to go as long as it works for your use case.<br>
<br>
Cheers,<br>
/Manuel<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> Cheers,<br>
><br>
> Victor<br>
><br>
><br>
><br>
>><br>
>> ><br>
>> ><br>
>> > 2012/8/17 Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>> >><br>
>> >> Hi Victor,<br>
>> >><br>
>> >> if I remember correctly we've been talking in IRC - have all your<br>
>> >> issues been resolved?<br>
>> >><br>
>> >> Cheers,<br>
>> >> /Manuel<br>
>> >><br>
>> >> On Thu, Aug 9, 2012 at 5:22 AM, Victor Vicente de Carvalho<br>
>> >> <<a href="mailto:victor.v.carvalho@gmail.com">victor.v.carvalho@gmail.com</a>> wrote:<br>
>> >> > Hello,<br>
>> >> ><br>
>> >> > In the past month I've been working in a clang tool to refactor the<br>
>> >> > fields<br>
>> >> > which are declared on a class constructor to match the order declared<br>
>> >> > on<br>
>> >> > the<br>
>> >> > header file, hence resolving the annoying Wreorder warning. After a<br>
>> >> > series<br>
>> >> > of difficulties finally I have something that is close to the ideal,<br>
>> >> > and<br>
>> >> > would like to someone more acquainted with clang internals to check<br>
>> >> > if<br>
>> >> > it's<br>
>> >> > the best form to implement this.<br>
>> >> ><br>
>> >> > Also, I've got some problems that are still remaining:<br>
>> >> ><br>
>> >> > 1 - I've been following the samples I've managed to find through the<br>
>> >> > internet, but every time I try to run the tool using the following<br>
>> >> > command<br>
>> >> > line (I'm intentionally not providing any include paths as I don't<br>
>> >> > want<br>
>> >> > to<br>
>> >> > std to be parsed):<br>
>> >> ><br>
>> >> > ../cpp-fixya <SOURCE.CPP> --<br>
>> >> ><br>
>> >> > I get the following errros:<br>
>> >> ><br>
>> >> > <premain>: CommandLine Error: Argument 'version' defined more than<br>
>> >> > once!<br>
>> >> > <premain>: CommandLine Error: Argument 'print-all-options' defined<br>
>> >> > more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > <premain>: CommandLine Error: Argument 'print-options' defined more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > <premain>: CommandLine Error: Argument 'help-hidden' defined more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > <premain>: CommandLine Error: Argument 'help' defined more than once!<br>
>> >> > <premain>: CommandLine Error: Argument 'debug-only' defined more than<br>
>> >> > once!<br>
>> >> > <premain>: CommandLine Error: Argument 'debug-buffer-size' defined<br>
>> >> > more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > <premain>: CommandLine Error: Argument 'debug' defined more than<br>
>> >> > once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'version' defined more than<br>
>> >> > once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'print-all-options' defined<br>
>> >> > more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'print-options' defined more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'help-hidden' defined more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'help' defined more than once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'debug-only' defined more than<br>
>> >> > once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'debug-buffer-size' defined<br>
>> >> > more<br>
>> >> > than<br>
>> >> > once!<br>
>> >> > cpp-fixya: CommandLine Error: Argument 'debug' defined more than<br>
>> >> > once!<br>
>> >> ><br>
>> >> ><br>
>> >> > 2 - I haven't found a way to determine the class name for nested<br>
>> >> > classes.<br>
>> >> > How would be the best way to detect and generate the correct naming<br>
>> >> > to a<br>
>> >> > nested class? I've been using:<br>
>> >> ><br>
>> >> > d->getNameAsString()<br>
>> >> ><br>
>> >> > but it only returns the enclosing class<br>
>> >> ><br>
>> >> ><br>
>> >> > 3 - How do I determine if a class declaration is on a header or a<br>
>> >> > source<br>
>> >> > file? I mean that because if it's a source-only declaration (i.e.<br>
>> >> > private<br>
>> >> > classes) the c++ compiler wont accept constructors in the form:<br>
>> >> ><br>
>> >> > Constructor::Constructor() : <init_declrs> {<br>
>> >> > }<br>
>> >> ><br>
>> >> > only in the form:<br>
>> >> ><br>
>> >> > Constructor() : <init_declrs> {<br>
>> >> > }<br>
>> >> ><br>
>> >> ><br>
>> >> > Here is the code:<br>
>> >> ><br>
>> >> > <a href="https://gist.github.com/3300494" target="_blank">https://gist.github.com/3300494</a><br>
>> >> ><br>
>> >> ><br>
>> >> > Thanks in advance,<br>
>> >> ><br>
>> >> ><br>
>> >> > Victor<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > _______________________________________________<br>
>> >> > cfe-dev mailing list<br>
>> >> > <a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
>> >> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
>> >> ><br>
>> ><br>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br></div>