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

[Manuel Klimek]

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.

> 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

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