<div class="gmail_quote">On Tue, Jul 17, 2012 at 11:40 AM, Jonathan Sauer <span dir="ltr"><<a href="mailto:jonathan.sauer@gmx.de" target="_blank">jonathan.sauer@gmx.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello,<br>
<br>
I have attached a shiny new patch :-)<br>
<div class="im"><br>
> As noted in the PR:<br>
><br>
> This is definitely the right approach. A few specific comments:<br>
><br>
>   - This change should affect the the EmitGCMemmoveCollectable call as well,<br>
> since the same bug exists there for Objective-C++ GC<br>
<br>
</div>Done.<br>
<div class="im"><br>
>   - There are other CreateMemCpy calls in IR generation that likely need this<br>
> same computation.<br>
<br>
</div>I looked at them, but as far as I could tell none of them are tasked with copying<br>
structs during assignment.<br>
<div class="im"><br>
> Please factor this computation into a separate routine and<br>
> update those callers<br>
<br>
</div>Done. As of now I did not include memoization of the result. If compile-time<br>
performance regresses, I can add it.<br>
<div class="im"><br>
>   - Please include a test case that generates IR and checks that it does the<br>
> right thing<br>
<br>
</div>Done. I hope the test file I expanded is the correct one.<br>
<br>
The only strange thing (I left a FIXME in the test case) is that a struct derived<br>
from a POD is not considered to be a POD (__is_pod also returns false in that case).<br>
I'm unsure whether this is correct (my reading of the standard says no).<br>
<br>
Also, was there a particular reason why the three isPOD-methods in Type originally<br>
got passed a non-const ASTContext instead of a const one?<br></blockquote><div><br></div><div>Checking isPODType here is not correct. The relevant term in the ABI document is "POD for the purpose of layout", not "POD", and means something subtly different (critically, it does not depend on whether we are in C++98 or C++11 mode). This term is defined in the ABI document.</div>
</div>