[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rsmith closed this revision. rsmith added a comment. Committed as r310401. Repository: rL LLVM https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rnk added inline comments. Comment at: test/CodeGenCXX/uncopyable-args.cpp:101 + +// In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC 2015, it is. +// WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64 rsmith wrote: > rnk wrote: > > Oh dear. :( > Can you check that MSVC 2013 is compatible with the code we produce here? > (I've checked 2015 passes this indirectly on Compiler Explorer.) Yes, 2013 passes this object directly as we do here. Repository: rL LLVM https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rsmith added inline comments.
Comment at: include/clang/AST/DeclCXX.h:420
+/// \brief True if this class has at least one non-deleted copy or move
+/// constructor. That would allow passing it by registers.
rnk wrote:
> Isn't this "... at least one *trivial*, non-deleted copy or move
> constructor..."?
Changed to:
```
/// \brief True if this class can be passed in a non-address-preserving
/// fashion (such as in registers) according to the C++ language rules.
/// This does not imply anything about how the ABI in use will actually
/// pass an object of this class.
```
Comment at: include/clang/AST/DeclCXX.h:827
+return data().DefaultedCopyConstructorIsDeleted;
+ }
+ /// \brief \c true if a defaulted move constructor for this class would be
v.g.vassilev wrote:
> Is there a reason for not keeping the default (for the file) 1 empty line
> between methods? Looks like if we add one new line before and after
> `hasSimpleMoveAssignment` is will be all consistent.
Done. (I was following the local style, but you're right that we don't do this
elsewhere in the class outside the `hasSimple` functions, excluding groups of
methods that are much more closely tied together such as `*_begin`/`*_end`.)
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:836
-// If this is true, the implicit copy constructor that Sema would have
-// created would not be deleted. FIXME: We should provide a more direct way
-// for CodeGen to ask whether the constructor was deleted.
-if (!RD->hasUserDeclaredCopyConstructor() &&
-!RD->hasUserDeclaredMoveConstructor() &&
-!RD->needsOverloadResolutionForMoveConstructor() &&
-!RD->hasUserDeclaredMoveAssignment() &&
-!RD->needsOverloadResolutionForMoveAssignment())
- return RAA_Default;
-
-// Otherwise, Sema should have created an implicit copy constructor if
-// needed.
-assert(!RD->needsImplicitCopyConstructor());
-
-// We have to make sure the trivial copy constructor isn't deleted.
-for (const CXXConstructorDecl *CD : RD->ctors()) {
- if (CD->isCopyConstructor()) {
-assert(CD->isTrivial());
-// We had at least one undeleted trivial copy ctor. Return directly.
-if (!CD->isDeleted())
- return RAA_Default;
+// Win64 passes objects with non-deleted, non-trivial copy ctors
indirectly.
+//
rnk wrote:
> This doesn't seem to match what we've computing, and it doesn't seem quite
> right. MSVC will pass a class with deleted, trivial copy ctors indirectly.
> Would it be correct to rephrase like this?
> "If RD has at least one trivial, non-deleted copy constructor, it is passed
> directly. Otherwise, it is passed indirectly."
You're right. I think "it is passed directly" is overspecifying, though, so how
about:
```
// If a class has at least one non-deleted, trivial copy constructor, it
// is passed according to the C ABI. Otherwise, it is passed indirectly.
```
Comment at: lib/Sema/SemaDeclCXX.cpp:5731
+/// registers, per C++ [class.temporary]p3.
+static bool computeCanPassInRegisters(Sema , CXXRecordDecl *D) {
+ if (D->isDependentType() || D->isInvalidDecl())
v.g.vassilev wrote:
> It would be very useful if we somehow assert if this function is called
> before the class triviality is computed?
Many of the functions we unconditionally call below will assert if the class
does not have a complete definition (eg, `needsImplicitCopyConstructor`).
Comment at: test/CodeGenCXX/uncopyable-args.cpp:101
+
+// In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC
2015, it is.
+// WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64
rnk wrote:
> Oh dear. :(
Can you check that MSVC 2013 is compatible with the code we produce here? (I've
checked 2015 passes this indirectly on Compiler Explorer.)
Repository:
rL LLVM
https://reviews.llvm.org/D35056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D35056#834705, @rnk wrote:
> In https://reviews.llvm.org/D35056#834689, @rsmith wrote:
>
> > I also removed some incorrect assumptions from the Win64 ABI code; this
> > changed the behavior of one testcase from uncopyable-args.cpp
> > (`implicitly_deleted_copy_ctor::A` is now passed indirect).
>
>
> That's probably not correct, let me take a look... I remember breaking the
> rules for small types here.
Nevermind, everything looks good there. Thanks for untangling the mess. I only
have comments on comments.
Comment at: include/clang/AST/DeclCXX.h:420
+/// \brief True if this class has at least one non-deleted copy or move
+/// constructor. That would allow passing it by registers.
Isn't this "... at least one *trivial*, non-deleted copy or move
constructor..."?
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:836
-// If this is true, the implicit copy constructor that Sema would have
-// created would not be deleted. FIXME: We should provide a more direct way
-// for CodeGen to ask whether the constructor was deleted.
-if (!RD->hasUserDeclaredCopyConstructor() &&
-!RD->hasUserDeclaredMoveConstructor() &&
-!RD->needsOverloadResolutionForMoveConstructor() &&
-!RD->hasUserDeclaredMoveAssignment() &&
-!RD->needsOverloadResolutionForMoveAssignment())
- return RAA_Default;
-
-// Otherwise, Sema should have created an implicit copy constructor if
-// needed.
-assert(!RD->needsImplicitCopyConstructor());
-
-// We have to make sure the trivial copy constructor isn't deleted.
-for (const CXXConstructorDecl *CD : RD->ctors()) {
- if (CD->isCopyConstructor()) {
-assert(CD->isTrivial());
-// We had at least one undeleted trivial copy ctor. Return directly.
-if (!CD->isDeleted())
- return RAA_Default;
+// Win64 passes objects with non-deleted, non-trivial copy ctors
indirectly.
+//
This doesn't seem to match what we've computing, and it doesn't seem quite
right. MSVC will pass a class with deleted, trivial copy ctors indirectly.
Would it be correct to rephrase like this?
"If RD has at least one trivial, non-deleted copy constructor, it is passed
directly. Otherwise, it is passed indirectly."
Comment at: test/CodeGenCXX/uncopyable-args.cpp:101
+
+// In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC
2015, it is.
+// WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64
Oh dear. :(
Repository:
rL LLVM
https://reviews.llvm.org/D35056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev added a comment.
I do not feel qualified enough to review this patch but I added few minor
comments.
Comment at: include/clang/AST/DeclCXX.h:827
+return data().DefaultedCopyConstructorIsDeleted;
+ }
+ /// \brief \c true if a defaulted move constructor for this class would be
Is there a reason for not keeping the default (for the file) 1 empty line
between methods? Looks like if we add one new line before and after
`hasSimpleMoveAssignment` is will be all consistent.
Comment at: lib/Sema/SemaDeclCXX.cpp:5731
+/// registers, per C++ [class.temporary]p3.
+static bool computeCanPassInRegisters(Sema , CXXRecordDecl *D) {
+ if (D->isDependentType() || D->isInvalidDecl())
It would be very useful if we somehow assert if this function is called before
the class triviality is computed?
Repository:
rL LLVM
https://reviews.llvm.org/D35056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev added a comment. And thanks for working on this!! Repository: rL LLVM https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rsmith added a comment. In https://reviews.llvm.org/D35056#834705, @rnk wrote: > In https://reviews.llvm.org/D35056#834689, @rsmith wrote: > > > I also removed some incorrect assumptions from the Win64 ABI code; this > > changed the behavior of one testcase from uncopyable-args.cpp > > (`implicitly_deleted_copy_ctor::A` is now passed indirect). > > > That's probably not correct, let me take a look... I remember breaking the > rules for small types here. I forgot to say: the new behavior matches MSVC. (The immediate bug was that we didn't consider the possibility that a user-declared copy assignment operator could cause the implicit copy constructor to be deleted, but there may have been other bugs also caused by this part of CodeGen attempting to work out deletedness for itself rather than asking.) Repository: rL LLVM https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rnk added a comment. In https://reviews.llvm.org/D35056#834689, @rsmith wrote: > I also removed some incorrect assumptions from the Win64 ABI code; this > changed the behavior of one testcase from uncopyable-args.cpp > (`implicitly_deleted_copy_ctor::A` is now passed indirect). That's probably not correct, let me take a look... I remember breaking the rules for small types here. Repository: rL LLVM https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rsmith updated this revision to Diff 110115.
rsmith edited the summary of this revision.
rsmith added a comment.
Herald added a subscriber: klimek.
Remove added calls to `DeclareImplicit*` and `ShouldDeleteSpecialMember`. In
their place, figure out whether an implicit special member would be deleted or
not by querying the AST.
This requires teaching `CXXRecordDecl` to track whether an implicit copy
constructor for a class would be deleted, in the same way we do for the move
constructor and move assignment operator; as with those other two cases, we
fall back to an "ask Sema" state if the computation is not simple, and in that
case `Sema` eagerly declares the special member in question to compute the
answer. As a result, this can cause us to declare some copy constructors that
we didn't declare previously, but in the important common cases we will still
declare them lazily.
I also removed some incorrect assumptions from the Win64 ABI code; this changed
the behavior of one testcase from uncopyable-args.cpp
(`implicitly_deleted_copy_ctor::A` is now passed indirect).
Repository:
rL LLVM
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1108,26 +1108,35 @@
}
TEST(ConstructorDeclaration, Kinds) {
- EXPECT_TRUE(matches("struct S { S(); };",
- cxxConstructorDecl(isDefaultConstructor(;
- EXPECT_TRUE(notMatches("struct S { S(); };",
- cxxConstructorDecl(isCopyConstructor(;
- EXPECT_TRUE(notMatches("struct S { S(); };",
- cxxConstructorDecl(isMoveConstructor(;
-
- EXPECT_TRUE(notMatches("struct S { S(const S&); };",
- cxxConstructorDecl(isDefaultConstructor(;
- EXPECT_TRUE(matches("struct S { S(const S&); };",
- cxxConstructorDecl(isCopyConstructor(;
- EXPECT_TRUE(notMatches("struct S { S(const S&); };",
- cxxConstructorDecl(isMoveConstructor(;
-
- EXPECT_TRUE(notMatches("struct S { S(S&&); };",
- cxxConstructorDecl(isDefaultConstructor(;
- EXPECT_TRUE(notMatches("struct S { S(S&&); };",
- cxxConstructorDecl(isCopyConstructor(;
- EXPECT_TRUE(matches("struct S { S(S&&); };",
- cxxConstructorDecl(isMoveConstructor(;
+ EXPECT_TRUE(matches(
+ "struct S { S(); };",
+ cxxConstructorDecl(isDefaultConstructor(), unless(isImplicit();
+ EXPECT_TRUE(notMatches(
+ "struct S { S(); };",
+ cxxConstructorDecl(isCopyConstructor(), unless(isImplicit();
+ EXPECT_TRUE(notMatches(
+ "struct S { S(); };",
+ cxxConstructorDecl(isMoveConstructor(), unless(isImplicit();
+
+ EXPECT_TRUE(notMatches(
+ "struct S { S(const S&); };",
+ cxxConstructorDecl(isDefaultConstructor(), unless(isImplicit();
+ EXPECT_TRUE(matches(
+ "struct S { S(const S&); };",
+ cxxConstructorDecl(isCopyConstructor(), unless(isImplicit();
+ EXPECT_TRUE(notMatches(
+ "struct S { S(const S&); };",
+ cxxConstructorDecl(isMoveConstructor(), unless(isImplicit();
+
+ EXPECT_TRUE(notMatches(
+ "struct S { S(S&&); };",
+ cxxConstructorDecl(isDefaultConstructor(), unless(isImplicit();
+ EXPECT_TRUE(notMatches(
+ "struct S { S(S&&); };",
+ cxxConstructorDecl(isCopyConstructor(), unless(isImplicit();
+ EXPECT_TRUE(matches(
+ "struct S { S(S&&); };",
+ cxxConstructorDecl(isMoveConstructor(), unless(isImplicit();
}
TEST(ConstructorDeclaration, IsUserProvided) {
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck %s -check-prefix=WIN64
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=18 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-18
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s -fms-compatibility -fms-compatibility-version=19 | FileCheck %s -check-prefix=WIN64 -check-prefix=WIN64-19
namespace trivial {
// Trivial structs should be passed
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rsmith added a comment. As requested by Vassil, I'm going to upload another version of this that avoids declaring implicit special members so frequently. https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rjmccall added inline comments.
Comment at: lib/Sema/SemaDeclCXX.cpp:5742
+// effect of performing a trivial copy of the type.
+bool Sema::CanPassInRegisters(CXXRecordDecl *D) {
+ if (D->isDependentType())
This should probably be called something like "computeCanPassInRegisters" to
discourage other code from calling it directly.
It should also just be a static function in this file unless it needs to be a
member for some access-control reason.
https://reviews.llvm.org/D35056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev updated this revision to Diff 109932.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.
We set the record's property denoting whether we can pass the decl by registers
as a last step of `Sema::CheckCompletedCXXClass`. We cannot do it any earlier
than that because we have not computed the triviality information.
This patch still eagerly defines too many implicit members. As discussed with
@rsmith, he will take care of the non-trivial refactoring of
`Sema::ShouldDeleteSpecialMember` to fix this regression.
This patch passes all tests in codegen but still fails a few which are
sensitive to the amount of implicit members being created:
Failing Tests (13):
Clang :: CodeCompletion/ordinary-name-cxx11.cpp
Clang :: Misc/ast-dump-color.cpp
Clang :: Misc/ast-dump-decl.cpp
Clang :: Misc/ast-dump-invalid.cpp
Clang-Unit ::
AST/./ASTTests/CXXMethodDecl.CXXMethodDeclWithNoExceptSpecification
Clang-Unit ::
ASTMatchers/./ASTMatchersTests/ConstructorDeclaration.IsImplicit
Clang-Unit :: ASTMatchers/./ASTMatchersTests/ConstructorDeclaration.Kinds
Clang-Unit :: ASTMatchers/./ASTMatchersTests/DeclarationMatcher.MatchNot
Clang-Unit :: ASTMatchers/./ASTMatchersTests/DeclarationMatcher.hasMethod
Clang-Unit ::
ASTMatchers/./ASTMatchersTests/DeclaratorDecl.MatchesDeclaratorDecls
Clang-Unit :: ASTMatchers/./ASTMatchersTests/Matcher.References
Clang-Unit :: ASTMatchers/./ASTMatchersTests/TypeMatcher.MatchesClassType
Clang-Unit ::
ASTMatchers/Dynamic/./DynamicASTMatchersTests/RegistryTest.VariadicOp
Expected Passes: 10786
Expected Failures : 17
Unsupported Tests : 195
Unexpected Failures: 13
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
include/clang/Sema/Sema.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-//
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rjmccall added inline comments. Comment at: lib/CodeGen/CGCXXABI.cpp:43 if (RD->hasNonTrivialDestructor()) return false; v.g.vassilev wrote: > rjmccall wrote: > > Does canPassInRegisters() not subsume all of these earlier checks? > No, if I remove them here I get a lot of test failures. I cannot move them > (yet?) in Sema, because I need to call `Sema::CheckCompletedCXXClass` in > `Sema::ActOnFields` to compute the triviality of the decl. Only then it would > be safe move these checks in `CanPassInRegisters`. I feel it's pretty unfortunate to name a property "canPassInRegisters" when it actually means something quite different. People will expect this to mean what it says. https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev added inline comments. Comment at: lib/CodeGen/CGCXXABI.cpp:43 if (RD->hasNonTrivialDestructor()) return false; rjmccall wrote: > Does canPassInRegisters() not subsume all of these earlier checks? No, if I remove them here I get a lot of test failures. I cannot move them (yet?) in Sema, because I need to call `Sema::CheckCompletedCXXClass` in `Sema::ActOnFields` to compute the triviality of the decl. Only then it would be safe move these checks in `CanPassInRegisters`. https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rjmccall added inline comments. Comment at: lib/CodeGen/CGCXXABI.cpp:43 if (RD->hasNonTrivialDestructor()) return false; Does canPassInRegisters() not subsume all of these earlier checks? https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev updated this revision to Diff 108556.
v.g.vassilev added a comment.
Move back the triviality checks in CGCXXABI. Explain why.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
}
@@ -195,12 +191,10 @@
void bar() {
foo({});
}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev updated this revision to Diff 108535.
v.g.vassilev added a comment.
Put back accidentally removed test case.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
}
@@ -195,12 +191,10 @@
void bar() {
foo({});
}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5885,6 +5885,7 @@
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:14841-14852
+ // If a move constructor or move assignment operator was declared, the
+ // default copy constructors are implicitly deleted, except in one case
+ // related to compatibility with MSVC pre-2015.
+ if (CXXRD->hasUserDeclaredMoveConstructor())
+CopyOrMoveDeleted = true;
+ if (CXXRD->hasUserDeclaredMoveAssignment()) {
+const LangOptions = SemaRef.getASTContext().getLangOpts();
rsmith wrote:
> Do we need this? The above code will have already declared as deleted the
> relevant operator, so this seems like it can never trigger.
You are right, we do not need it.
https://reviews.llvm.org/D35056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev updated this revision to Diff 108533.
v.g.vassilev marked 6 inline comments as done.
v.g.vassilev added a comment.
Address some of the comments.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -1,87 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck %s -check-prefix=WIN64
-namespace trivial {
-// Trivial structs should be passed directly.
-struct A {
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// CHECK-LABEL: define void @_ZN7trivial3barEv()
-// CHECK: alloca %"struct.trivial::A"
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN7trivial3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN7trivial3fooENS_1AE(i8*)
-
-// WIN64-LABEL: declare void @"\01?foo@trivial@@YAXUA@1@@Z"(i64)
-}
-
-namespace default_ctor {
-struct A {
- A();
- void *p;
-};
-void foo(A);
-void bar() {
- // Core issue 1590. We can pass this type in registers, even though C++
- // normally doesn't permit copies when using braced initialization.
- foo({});
-}
-// CHECK-LABEL: define void @_ZN12default_ctor3barEv()
-// CHECK: alloca %"struct.default_ctor::A"
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN12default_ctor3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN12default_ctor3fooENS_1AE(i8*)
-
-// WIN64-LABEL: declare void @"\01?foo@default_ctor@@YAXUA@1@@Z"(i64)
-}
-
-namespace move_ctor {
-// The presence of a move constructor implicitly deletes the trivial copy ctor
-// and means that we have to pass this struct by address.
-struct A {
- A();
- A(A &);
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
-}
-
-namespace all_deleted {
-struct A {
- A();
- A(const A ) = delete;
- A(A &) = delete;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
-}
namespace implicitly_deleted {
struct A {
@@ -93,188 +12,12 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
-}
-
-namespace one_deleted {
-struct A {
- A();
- A(A &) = delete;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
-}
-
-namespace copy_defaulted {
-struct A {
- A();
- A(const A ) = default;
- A(A &) = delete;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// CHECK-LABEL: define void @_ZN14copy_defaulted3barEv()
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN14copy_defaulted3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN14copy_defaulted3fooENS_1AE(i8*)
+// CHECK-NOT: call
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rsmith added inline comments.
Comment at: lib/CodeGen/CGCXXABI.cpp:33-47
+ // See also Sema::ShouldDeleteSpecialMember. These two functions
+ // should be kept consistent.
+
// If RD has a non-trivial move or copy constructor, we cannot copy the
// argument.
if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor())
return false;
Perhaps it would make sense to put this whole "does the language permit passing
in registers?" calculation in Sema and store that in the DefinitionData rather
than storing the "HasNonDeletedCopyOrMoveConstructor" flag.
Comment at: lib/Sema/SemaDecl.cpp:14816-14817
+static bool HasNonDeletedCopyOrMoveCtor(CXXRecordDecl *CXXRD, Sema ) {
+ // FIXME: Handle the cases in which CXXRD is dependent. We need to
instantiate
+ // the class to figure out if the constructor should be deleted.
+ assert(!CXXRD->isDependentType() && "Not implemented yet");
This seems like a good motivation for the flag to be a "can pass in registers"
flag: that makes it much more obvious that it's meaningless on a dependent
class, since such a class will never be passed / returned at all.
Comment at: lib/Sema/SemaDecl.cpp:14821-14822
+CXXConstructorDecl *CopyCtor =
SemaRef.DeclareImplicitCopyConstructor(CXXRD);
+if (SemaRef.ShouldDeleteSpecialMember(CopyCtor, Sema::CXXCopyConstructor))
+ SemaRef.SetDeclDeleted(CopyCtor, CXXRD->getLocation());
+ }
You don't need to do this: triggering the declaration will delete the member if
relevant.
However, we should refactor `ShouldDeleteSpecialMember` so that it can be
called without actually having built a special member declaration: this patch
removes all laziness in declaring copy or move constructors.
Comment at: lib/Sema/SemaDecl.cpp:14825
+
+ if (CXXRD->needsImplicitMoveConstructor()) {
+CXXConstructorDecl *MoveCtor =
SemaRef.DeclareImplicitMoveConstructor(CXXRD);
This needs to be guarded by a `getLangOpts().CPlusPlus11` check.
Comment at: lib/Sema/SemaDecl.cpp:14831-14839
+ bool CopyOrMoveDeleted = false;
+ for (const CXXConstructorDecl *CD : CXXRD->ctors()) {
+if (CD->isMoveConstructor() || CD->isCopyConstructor()) {
+ if (!CD->isDeleted()) {
+return true;
+ }
+ CopyOrMoveDeleted = true;
We should do this check before we call `ShouldDeleteSpecialMember` to get the
benefit of the early exit. If we switch to tracking the ABI flag, we can also
bail out early if we see a non-trivial copy ctor / move ctor / dtor.
Comment at: lib/Sema/SemaDecl.cpp:14841-14852
+ // If a move constructor or move assignment operator was declared, the
+ // default copy constructors are implicitly deleted, except in one case
+ // related to compatibility with MSVC pre-2015.
+ if (CXXRD->hasUserDeclaredMoveConstructor())
+CopyOrMoveDeleted = true;
+ if (CXXRD->hasUserDeclaredMoveAssignment()) {
+const LangOptions = SemaRef.getASTContext().getLangOpts();
Do we need this? The above code will have already declared as deleted the
relevant operator, so this seems like it can never trigger.
Comment at: lib/Sema/SemaDecl.cpp:15147-15148
if (!Completed)
Record->completeDefinition();
This call also needs to pass in the relevant flag.
https://reviews.llvm.org/D35056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev updated this revision to Diff 108505.
v.g.vassilev added a comment.
Move the checks in Sema.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
}
@@ -195,12 +191,10 @@
void bar() {
foo({});
}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5885,6 +5885,7 @@
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rsmith added inline comments. Comment at: lib/AST/DeclCXX.cpp:1476 + + data().HasNonDeletedCopyOrMoveConstructor = !CopyOrMoveDeleted; } This is wrong. "Has a non-deleted copy or move constructor" is not the same thing as "does not have a deleted copy or move constructor". And you will also need to check for the case where the class notionally has such a constructor, but where we have lazily not yet declared it -- in that case, you may need to do arbitrary work (including template instantiation etc) to figure out whether the constructor should be deleted. Please move this check into `Sema` and pass a bool to `completeDefinition` with the result of the check. That way you will have the tools available to compute the correct value in the case where you need to trigger the declaration of a constructor to determine the right answer. https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rjmccall added a comment. Thank you, I like this approach much better, and the IRGen changes seem fine to me. I'd like to defer to someone else (probably Richard) to review whether the changes to completeDefinition() are correct; I'm not up-to-date with how we handle lazy declarations. https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev updated this revision to Diff 105776.
v.g.vassilev added a comment.
Remove accidentally added change in SemaDecl.cpp
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
}
@@ -195,12 +191,10 @@
void bar() {
foo({});
}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5885,6 +5885,7 @@
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev updated this revision to Diff 105775.
v.g.vassilev added a comment.
Compute only once if there is a non-deleted move or copy constructor and store
them in the `DefinitionData` of the `CXXRecordDecl`.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
}
@@ -195,12 +191,10 @@
void bar() {
foo({});
}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
}
Index: lib/Serialization/ASTWriter.cpp
===
---
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rnk added a comment. I discussed this with @rsmith and we think the correct fix is to update the DefinitionData bits computed when adding special members. I haven't reviewed this patch in detail, but we came to the conclusion that Clang's optimization to avoid implicit special member declarations in most situations makes it impossible to get the correct answer in all situations here. The explicit loop over `RD->ctors()` is a bug in itself, because important ones may not be there. I'm never able to remember the details, but we believe that's the "right" fix. This would change the values reported by some of the older pre-c++11 GCC type traits (the __has_* ones, I think), but modern versions of libstdc++ no longer use them. We're hesitant to accept a workaround in the meantime because it means we'll have to fix the bug again later and create another ABI break with ourselves. However, we shouldn't let the perfect be the enemy of the good. In practice, I suspect we are not very good about maintaining ABI stability in corner cases this deep. Repository: rL LLVM https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
rjmccall added a comment. If this is a common algorithm across all ABIs, can we just put Sema / the AST in charge of computing it? It's not a cheap thing to recompute retroactively, especially for an imported type, and it seems like it's easily derived from the things that go into DerivedData. Repository: rL LLVM https://reviews.llvm.org/D35056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor
v.g.vassilev created this revision.
Patch by Bernd Schmidt!
Fixes PR19668 and PR23034.
Repository:
rL LLVM
https://reviews.llvm.org/D35056
Files:
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
}
@@ -195,12 +191,10 @@
void bar() {
foo({});
}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
}
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -63,11 +63,8 @@
bool classifyReturnType(CGFunctionInfo ) const override;
RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
-// Structures with either a non-trivial destructor or a
