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

[James Dennett]

On Sun, Aug 19, 2012 at 12:15 AM, Philip Craig <philipjcraig at gmail.com> wrote:
> On Sun, Aug 19, 2012 at 1:52 PM, Victor Vicente de Carvalho
> <victor.v.carvalho at gmail.com> wrote:
>> Hi Manuel,
>>
>> my rewrite basing only in source replacements is complete, but now I'm
>> facing some strange side effects on some files. Consider the example:
>>
>> header ----------
>>
>> #include <B.h> // B defines a simple class named B
>>
>> class A {
>> public:
>>   A();
>>
>>   float x;
>>   B b;
>> };
>>
>> source ---------------
>>
>> A::A() : x(0) {
>> }
>>
>>
>> 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.
>>
>> 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.
>
> The AST for the constructor looks like this:
>
>   CXXConstructorDecl <A.cc:2:1-3:1>
>     NestedNameSpecifier <2:1-2> A::
>       RecordType <2:1>
>     DeclarationName <2:4> A
>     FunctionProtoType <-6>
>       BuiltinType void <>
>     CXXCtorInitializer  <2:10-13>
>       FieldDeclRef <2:10> A::x
>       ImplicitCastExpr <2:12>
>         IntegerLiteral <2:12> 0
>     CXXCtorInitializer  <2:4>
>       FieldDeclRef <2:4> A::b
>     CompoundStmt <2:15-3:1>
>
> Reversing the order of x and b just reverses the order of the two
> CXXCtorInitializer in the AST.
>
> So it looks like clang always creates implicit initializers for
> members that aren't explicitly initialized, but it has no way of
> flagging them as implicit. I don't know the expected way of
> determining which are implicit, maybe checking for a NULL getInit().

CXXCtorInitializer::isWritten "Returns true if this initializer is
explicitly written in the source code."

-- James