[cfe-dev] Criticism over a clang tool to reorder fields on constructor declarations

[Manuel Klimek]

On Sat, Aug 18, 2012 at 12:16 AM, Victor Vicente de Carvalho
<victor.v.carvalho at gmail.com> wrote:
> 2012/8/17 Manuel Klimek <klimek at google.com>
>>
>> On Fri, Aug 17, 2012 at 9:34 PM, Victor Vicente de Carvalho
>> <victor.v.carvalho at gmail.com> wrote:
>> > Hello Manuel,
>> >
>> > Sadly, no. I'm struggling with some problems now:
>> >
>> > 1 - How do I detect and generate the class name for nested classes?  I'm
>> > using  CXXConstructorDecl getNameAsString() but it only returns to me
>> > the
>> > name of the enclosing class.
>>
>> As Jends said, getQualifiedNameAsString.
>>
>
> Thanks! It worked
>
>
>>
>> > 2 - I would like to not replace constructor initializers that were set
>> > up
>> > with macros with their values:
>> >
>> > #define MACRO 10
>> >
>> > Class::Class() : myval(MACRO)
>> >
>> > after I rewrite the outputted code is:
>> >
>> > Class::Class() : mvyal(10)
>> >
>> > Is there a way to avoid this? I've tried to use
>> > Compiler.getPreprocessor().SetMacroExpansionOnlyInDirectives() but this
>> > simply removes the myval definition from the generated AST
>>
>> Source locations have the notion of an expansion and a spelling
>> location. To get the code that was originally written, you'll need to
>> fiddle around with those. In your case, MACRO is the expansion
>> location of the literal 10.
>>
>> > 3 - Variables that are default initialized on the header are stored with
>> > it.
>> > This is a problem as the compiler complains when I rewrite the source
>> > file
>> > with the initialization on the source but it was previously defined on
>> > the
>> > header. Is there to detect if the default initializer is on the method
>> > definition or in the method declaration?
>>
>> I would assume so - I don't where to look of the top of my head though.
>>
>> >
>> > 4 - I've found a bug on the DeclPrinter class, and fixed it. The
>> > attachment
>> > is the patch.
>>
>> I suggest you send an email to cfe-commits with the subject [PATCH]
>> Fix problem with DeclPrinter, containing the patch and what was wrong
>> (and ideally an automated regression test ;)
>>
>> >
>> > 5 - The current way i'm using to replace the old constructor declaration
>> > with the new declaration is with a new range:
>> >
>> > SourceRange rng(d->getSourceRange().getBegin(),
>> > d->getBody()->getSourceRange().getBegin().getLocWithOffset(-1));
>> >
>> > This was the only way I've managed to get the correct end of the end of
>> > the
>> > initializer list. This is an inconvenience as I would like to not
>> > process
>> > the body and hence make the tool faster, but if I disable body parsing I
>> > can't get its SourceRange.
>>
>> As far as I understand (and I might well be wrong there) skipping body
>> parsing has some very concrete use cases, and source-to-source
>> transformations is not on of them...
>>
>> Cheers,
>> /Manuel
>>
>
> Manuel, I think I've been thinking the wrong way. As the only thing I need
> is to reorder the initialization, I do not need to mess with the original
> source like I was doing and just concatenate the reordered, original
> CXXCtorInitializer string data. So I've been wondering how I'm supposed to
> recover the source string based on a SourceLocation. Should I use
> SourceManager's getCharacterData? There is a way to recover the entire
> string based on a sourceRange?

RefactoringTool in lib/Tooling has some examples of how to deal with
that. They're not perfect yet, as source locations can be a little
fiddly to handle. In general, SourceManager's getCharacterData sounds
like a good way to go as long as it works for your use case.

Cheers,
/Manuel

>
> Cheers,
>
> Victor
>
>
>
>>
>> >
>> >
>> > 2012/8/17 Manuel Klimek <klimek at google.com>
>> >>
>> >> Hi Victor,
>> >>
>> >> if I remember correctly we've been talking in IRC - have all your
>> >> issues been resolved?
>> >>
>> >> Cheers,
>> >> /Manuel
>> >>
>> >> On Thu, Aug 9, 2012 at 5:22 AM, Victor Vicente de Carvalho
>> >> <victor.v.carvalho at gmail.com> wrote:
>> >> > Hello,
>> >> >
>> >> > In the past month I've been working in a clang tool to refactor the
>> >> > fields
>> >> > which are declared on a class constructor to match the order declared
>> >> > on
>> >> > the
>> >> > header file, hence resolving the annoying Wreorder warning. After a
>> >> > series
>> >> > of difficulties finally I have something that is close to the ideal,
>> >> > and
>> >> > would like to someone more acquainted with clang internals to check
>> >> > if
>> >> > it's
>> >> > the best form to implement this.
>> >> >
>> >> > Also, I've got some problems that are still remaining:
>> >> >
>> >> > 1 - I've been following the samples I've managed to find through the
>> >> > internet, but every time I try to run the tool using the following
>> >> > command
>> >> > line (I'm intentionally not providing any include paths as I don't
>> >> > want
>> >> > to
>> >> > std to be parsed):
>> >> >
>> >> > ../cpp-fixya <SOURCE.CPP>  --
>> >> >
>> >> > I get the following errros:
>> >> >
>> >> > <premain>: CommandLine Error: Argument 'version' defined more than
>> >> > once!
>> >> > <premain>: CommandLine Error: Argument 'print-all-options' defined
>> >> > more
>> >> > than
>> >> > once!
>> >> > <premain>: CommandLine Error: Argument 'print-options' defined more
>> >> > than
>> >> > once!
>> >> > <premain>: CommandLine Error: Argument 'help-hidden' defined more
>> >> > than
>> >> > once!
>> >> > <premain>: CommandLine Error: Argument 'help' defined more than once!
>> >> > <premain>: CommandLine Error: Argument 'debug-only' defined more than
>> >> > once!
>> >> > <premain>: CommandLine Error: Argument 'debug-buffer-size' defined
>> >> > more
>> >> > than
>> >> > once!
>> >> > <premain>: CommandLine Error: Argument 'debug' defined more than
>> >> > once!
>> >> > cpp-fixya: CommandLine Error: Argument 'version' defined more than
>> >> > once!
>> >> > cpp-fixya: CommandLine Error: Argument 'print-all-options' defined
>> >> > more
>> >> > than
>> >> > once!
>> >> > cpp-fixya: CommandLine Error: Argument 'print-options' defined more
>> >> > than
>> >> > once!
>> >> > cpp-fixya: CommandLine Error: Argument 'help-hidden' defined more
>> >> > than
>> >> > once!
>> >> > cpp-fixya: CommandLine Error: Argument 'help' defined more than once!
>> >> > cpp-fixya: CommandLine Error: Argument 'debug-only' defined more than
>> >> > once!
>> >> > cpp-fixya: CommandLine Error: Argument 'debug-buffer-size' defined
>> >> > more
>> >> > than
>> >> > once!
>> >> > cpp-fixya: CommandLine Error: Argument 'debug' defined more than
>> >> > once!
>> >> >
>> >> >
>> >> > 2 - I haven't found a way to determine the class name for nested
>> >> > classes.
>> >> > How would be the best way to detect and generate the correct naming
>> >> > to a
>> >> > nested class? I've been using:
>> >> >
>> >> > d->getNameAsString()
>> >> >
>> >> > but it only returns the enclosing class
>> >> >
>> >> >
>> >> > 3 - How do I determine if a class declaration is on a header or a
>> >> > source
>> >> > file? I mean that because if it's a source-only declaration (i.e.
>> >> > private
>> >> > classes) the c++ compiler wont accept constructors in the form:
>> >> >
>> >> > Constructor::Constructor()  : <init_declrs> {
>> >> > }
>> >> >
>> >> > only in the form:
>> >> >
>> >> > Constructor()  : <init_declrs> {
>> >> > }
>> >> >
>> >> >
>> >> > Here is the code:
>> >> >
>> >> > https://gist.github.com/3300494
>> >> >
>> >> >
>> >> > Thanks in advance,
>> >> >
>> >> >
>> >> > Victor
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > _______________________________________________
>> >> > cfe-dev mailing list
>> >> > cfe-dev at cs.uiuc.edu
>> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> >> >
>> >
>> >
>
>