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

Victor Vicente de Carvalho victor.v.carvalho at gmail.com
Sun Aug 19 17:31:24 PDT 2012


But why the source location for the first initializer is completely messed
up? I've just tried to dump the construct xml using clang -dump-ast-xml,
and that's what I've got:

<CXXConstructor ptr="0x4c664c0" name="Plane" previous="0x4c65000"
prototype="true">
  <FunctionProtoType ptr="0x4c64ae0" canonical="0x4c64ae0">
   <BuiltinType ptr="0x4c254a0" canonical="0x4c254a0"/>
   <parameters>
    <LValueReferenceType ptr="0x4c5df40" canonical="0x4c5df40">
     <QualType const="true">
      <RecordType ptr="0x4c5d820" canonical="0x4c5d820">
       <CXXRecord ref="0x4c5d790"/>
      </RecordType>
     </QualType>
    </LValueReferenceType>
    <BuiltinType ptr="0x4c25640" canonical="0x4c25640"/>
   </parameters>
  </FunctionProtoType>
  <ParmVar ptr="0x4c66270" name="normal" initstyle="c">
   <LValueReferenceType ptr="0x4c5df40" canonical="0x4c5df40">
    <QualType const="true">
     <RecordType ptr="0x4c5d820" canonical="0x4c5d820">
      <CXXRecord ref="0x4c5d790"/>
     </RecordType>
    </QualType>
   </LValueReferenceType>
  </ParmVar>
  <ParmVar ptr="0x4c662e0" name="d" initstyle="c">
   <BuiltinType ptr="0x4c25640" canonical="0x4c25640"/>
  </ParmVar>
  <Stmt>
(CompoundStmt 0x4c666d8 <Plane.cpp:27:55, line:29:1>)

  </Stmt>
 </CXXConstructor>

for the folowing constructor:

Plane::Plane (const Vector3& normal, float d) : d(d)  {
  this->normal.set(normal).nor();
}

Somehow I'm not being able to recover the initializer list, nor the correct
SourceLocation for it.


Victor

2012/8/19 Philip Craig <philipjcraig at gmail.com>

> On Mon, Aug 20, 2012 at 2:10 AM, James Dennett <james.dennett at gmail.com>
> wrote:
> > 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."
>
> Thanks, I missed that. I see RecursiveASTVisitor is already checking
> that, but only to skip over getInit(). Would it make sense for RAV to
> skip over the CXXCtorInitializer completely unless
> shouldVisitImplicitCode() is true?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120819/383d1c09/attachment.html>


More information about the cfe-dev mailing list