[PATCH] Don't copy objects with trivial, deleted copy ctors
Richard Smith
richard at metafoo.co.uk
Thu May 8 14:12:14 PDT 2014
> Richard, you had concerns that this loop over ctors was incorrect, and that
> I should really be making some change to CXXRecordDecl::DefinitionData.
> Can you imagine some test cases that might be problematic?
Something like:
struct A {
A(const A&) = default;
A(const A&, int = 0);
void *p;
};
struct B : A {};
void foo(B); // B should be passed indirect, because its copy ctor is deleted, but it's also trivial and not-yet-declared so we might not realize this.
> If so, do you think it's worth pursuing a more complete fix that
> updates the flags, or is it OK to accept this as an incremental fix?
It's an extremely obscure case, so the incremental fix seems reasonable to me.
================
Comment at: lib/CodeGen/CGCXXABI.cpp:49-50
@@ +48,4 @@
+
+ // If all trivial copy and move constructors are deleted, we cannot copy the
+ // argument.
+ return !(AnyDeleted && AllDeleted);
----------------
Please add a FIXME saying that we assume that all lazily-declared trivial copy/move constructors are not deleted, and that this assumption might not actually be true in some corner cases.
================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:65-66
@@ -65,1 +64,4 @@
+ return RAA_Indirect;
+ // If the class has a destructor, the ABI requires us to pass by address.
+ if (RD->hasNonTrivialDestructor())
return RAA_Indirect;
----------------
This should probably be part of the 'canCopy' check; the proposed fix for DR1590 also requires a trivial destructor.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:431-435
@@ +430,7 @@
+
+ // MSVC passes objects with a trivial, non-deleted copy ctor directly.
+ for (const CXXConstructorDecl *CD : RD->ctors()) {
+ if (CD->isCopyConstructor() && CD->isTrivial() && !CD->isDeleted())
+ return RAA_Default;
+ }
+
----------------
As an optimization, you could skip this if `!hasTrivialCopyConstructor()`.
http://reviews.llvm.org/D3660
More information about the cfe-commits
mailing list