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