On Tue, Sep 1, 2015 at 12:31 AM Olivier Goffart <ogoff...@kde.org> wrote:
> On Monday 31. August 2015 08:07:58 Manuel Klimek wrote:
> > On Sat, Aug 29, 2015 at 12:23 PM Olivier Goffart via cfe-commits <
> >
> > cfe-commits@lists.llvm.org> wrote:
> > > Hi,
> > >
> > > Please review the attached patch.
> > >
> > > In Sema::BuildCXXFunctionalCastExpr, if the class has a destructor, the
> > > Op.SrcExpr might be a CXXBindTemporaryExpr which we need to unwrap.
> > >
> > > In the testcase, the first new CHECK worked (because A does not have a
> > > destructor), but the second CHECK failed (did not include the last
> > > parenthese) because D has a destructor.
> > >
> > > I used dyn_cast_or_null just to be safe, becasue i don't know if it is
> > > possible for the BindExpr->getSubExpr() to be null.
> >
> > Do you know why the added test for A says 'class foo::A' instead of
> > 'foo::A' as the other tests do in that file?
>
> I don't know. It seems it has to do with the fully quallified name vs.
> normal
> name.
>
> For example, if you dumpt the AST of:
>
> namespace foo { class A {} *a1 = new foo::A , *a2 = new A; }
>
> You get:
>
> [...]
> | |-VarDecl 0x1c9a7a0 <col:17, col:43> col:29 a1 'class A *' cinit
> | | `-CXXNewExpr 0x1ce2648 <col:34, col:43> 'foo::A *'
> | | `-CXXConstructExpr 0x1ce2618 <col:38> 'foo::A':'class foo::A' 'void
> (void) throw()'
> | `-VarDecl 0x1ce26d0 <col:17, col:57> col:48 a2 'class A *' cinit
> | `-CXXNewExpr 0x1ce2768 <col:53, col:57> 'class foo::A *'
> | `-CXXConstructExpr 0x1ce2738 <col:57> 'class foo::A' 'void (void)
> throw()'
>
>
> As you can see, when the type of CXXNewExpr is fully quialified, the
> 'class'
> keyword is omited, but for a2, it prints the 'class' keyword.
> The printed type of CXXConstructExpr is even more wierd when it is fully
> qualified.
>
>
> I guess that's because of ElaboratedTypePolicyRAII in TypePrinter.cpp
>
> But this is irrelevant for this patch and the problem it's trying to solve.
> The reason I used 'using namespace foo' in the test was just so the line
> with
> A and D have the same size. I just copy pasted the thing without thinking
> about that.
>
Makes sense. In that case I think it looks good, adding Richard to
cross-check for the final approval.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
