[cfe-commits] [Patch][Review] Bug 13329 - Incorrect code generated for automatic assignment operator

Richard Smith richard at metafoo.co.uk
Tue Jul 17 14:02:20 PDT 2012


On Tue, Jul 17, 2012 at 11:40 AM, Jonathan Sauer <jonathan.sauer at gmx.de>wrote:

> Hello,
>
> I have attached a shiny new patch :-)
>
> > As noted in the PR:
> >
> > This is definitely the right approach. A few specific comments:
> >
> >   - This change should affect the the EmitGCMemmoveCollectable call as
> well,
> > since the same bug exists there for Objective-C++ GC
>
> Done.
>
> >   - There are other CreateMemCpy calls in IR generation that likely need
> this
> > same computation.
>
> I looked at them, but as far as I could tell none of them are tasked with
> copying
> structs during assignment.
>
> > Please factor this computation into a separate routine and
> > update those callers
>
> Done. As of now I did not include memoization of the result. If
> compile-time
> performance regresses, I can add it.
>
> >   - Please include a test case that generates IR and checks that it does
> the
> > right thing
>
> Done. I hope the test file I expanded is the correct one.
>
> The only strange thing (I left a FIXME in the test case) is that a struct
> derived
> from a POD is not considered to be a POD (__is_pod also returns false in
> that case).
> I'm unsure whether this is correct (my reading of the standard says no).
>
> Also, was there a particular reason why the three isPOD-methods in Type
> originally
> got passed a non-const ASTContext instead of a const one?
>

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120717/dca2576a/attachment.html>


More information about the cfe-commits mailing list