[PATCH] D45306: PR36992 don't overwrite virtual bases in tail padding

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D45306#1058792, @rjmccall wrote:

> Changing the requirements on the return-value slot would be an ABI break, no?


It would be, yes. But WG21 seems to have a taste for breaking changes at the moment, so who knows... I'm happy to remove `overlapForReturnValue` for now; it's easy enough to find these places again should the change ever happen.

> And it would force to us to treat all complete-object constructions as nvsize-limited unless they're constructions of known sizeof-sized allocations.

P0840 pushes us there already. Fortunately, we don't treat complete-object constructors specially in this regard currently, and no other compiler seems to do so either. However, when performing the trivial copy/move constructor -> `memcpy` optimization (which is I think the most interesting case for extending a store into the tail padding), we do know which object is being initialized, so we can take into account whether it could have something else allocated into its tail padding.



================
Comment at: CodeGen/CGDecl.cpp:1462
+                                              AggValueSlot::IsNotAliased,
+                                              AggValueSlot::DoesNotOverlap));
     }
----------------
rjmccall wrote:
> How is 'does not overlap' justified here?  Should this be propagated into the LValue?
That's what I get for trusting comments :)

No functional change from `DoesNotOverlap` here yet, though, because in practice this is always called with some kind of `VarDecl` (which is `DoesNotOverlap` because it's a complete object) or some kind of `FieldDecl` (which is `DoesNotOverlap` until P0840 lands).


https://reviews.llvm.org/D45306