[clang] [clang][Sema] Track trivial-relocatability as a type trait (PR #84621)
@@ -857,8 +881,13 @@ void CXXRecordDecl::addedMember(Decl *D) {
data().HasDeclaredCopyAssignmentWithConstParam = true;
}
-if (Method->isMoveAssignmentOperator())
+if (Method->isMoveAssignmentOperator()) {
SMKind |= SMF_MoveAssignment;
+}
+
+if (Method->isUserProvided() &&
+(Method->isCopyAssignment() || Method->isMoveAssignment()))
+ data().IsNaturallyTriviallyRelocatable = false;
thiagomacieira wrote:
Some solicited comments, from Qt maintainer.
I don't have a strong opinion on whether the presence of an assignment operator
should affect relocatability. Only "stupid types" will have a trivial copy/move
constructor and a non-trivial assignment operator that does something
different. Those types are inherently broken, because a move can be implemented
as either
```c++
tgt = std::move(src);
```
or as
```c++
std::destroy_at(&tgt);
std::construct_at(&tgt, std::move(src));
```
(provided `&tgt != &src`)
It is the container implementer's choice of which those two to use and we have
both being chosen in different frameworks, sometimes even both inside the same
framework. Therefore, any type that has visibly different behaviour for the two
implementations above is "stupid" and has bigger problems than its
relocatability.
That means it's a reasonable choice to say the presence of the assignment
operator does not affect the relocatability. But then it's also an equally
valid choice to say it does, with the benefit that one could find out that a
type is "stupid" before a release.
I do have a weak "leans to" choice though: for the first implementation of
trivial relocatability, we should err on the side of being more restrictive. We
can always be less so in the future, but going the other direction is more
problematic.
https://github.com/llvm/llvm-project/pull/84621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira added a comment. Ping? http://reviews.llvm.org/D14727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira added a comment. Ah, I see a comment. Sorry about that. I'll see if I can fix it, but I had no failures when I tried... http://reviews.llvm.org/D14727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira created this revision. thiagomacieira added a reviewer: cfe-commits. Some GCC 5 installations store the libstdc++ includes and GCC-specific files in paths without the minor part of the version number, such as /usr/include/c++/5 /usr/lib64/gcc/x86_64-suse-linux/5 http://reviews.llvm.org/D14727 Files: lib/Driver/ToolChains.cpp Index: lib/Driver/ToolChains.cpp === --- lib/Driver/ToolChains.cpp +++ lib/Driver/ToolChains.cpp @@ -1231,13 +1231,12 @@ if (First.first.getAsInteger(10, GoodVersion.Major) || GoodVersion.Major < 0) return BadVersion; GoodVersion.MajorStr = First.first.str(); - if (Second.first.getAsInteger(10, GoodVersion.Minor) || GoodVersion.Minor < 0) -return BadVersion; GoodVersion.MinorStr = Second.first.str(); // First look for a number prefix and parse that if present. Otherwise just // stash the entire patch string in the suffix, and leave the number // unspecified. This covers versions strings such as: + // 5 // 4.4 // 4.4.0 // 4.4.x Index: lib/Driver/ToolChains.cpp === --- lib/Driver/ToolChains.cpp +++ lib/Driver/ToolChains.cpp @@ -1231,13 +1231,12 @@ if (First.first.getAsInteger(10, GoodVersion.Major) || GoodVersion.Major < 0) return BadVersion; GoodVersion.MajorStr = First.first.str(); - if (Second.first.getAsInteger(10, GoodVersion.Minor) || GoodVersion.Minor < 0) -return BadVersion; GoodVersion.MinorStr = Second.first.str(); // First look for a number prefix and parse that if present. Otherwise just // stash the entire patch string in the suffix, and leave the number // unspecified. This covers versions strings such as: + // 5 // 4.4 // 4.4.0 // 4.4.x ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira updated this revision to Diff 40359.
thiagomacieira added a comment.
v2: fix accidental breakage of GCC 4
http://reviews.llvm.org/D14727
Files:
lib/Driver/ToolChains.cpp
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1230,14 +1230,17 @@
GCCVersion GoodVersion = {VersionText.str(), -1, -1, -1, "", "", ""};
if (First.first.getAsInteger(10, GoodVersion.Major) || GoodVersion.Major < 0)
return BadVersion;
- GoodVersion.MajorStr = First.first.str();
+ if (First.second.empty())
+return GoodVersion;
+
if (Second.first.getAsInteger(10, GoodVersion.Minor) || GoodVersion.Minor <
0)
return BadVersion;
GoodVersion.MinorStr = Second.first.str();
// First look for a number prefix and parse that if present. Otherwise just
// stash the entire patch string in the suffix, and leave the number
// unspecified. This covers versions strings such as:
+ // 5(handled above)
// 4.4
// 4.4.0
// 4.4.x
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1230,14 +1230,17 @@
GCCVersion GoodVersion = {VersionText.str(), -1, -1, -1, "", "", ""};
if (First.first.getAsInteger(10, GoodVersion.Major) || GoodVersion.Major < 0)
return BadVersion;
- GoodVersion.MajorStr = First.first.str();
+ if (First.second.empty())
+return GoodVersion;
+
if (Second.first.getAsInteger(10, GoodVersion.Minor) || GoodVersion.Minor < 0)
return BadVersion;
GoodVersion.MinorStr = Second.first.str();
// First look for a number prefix and parse that if present. Otherwise just
// stash the entire patch string in the suffix, and leave the number
// unspecified. This covers versions strings such as:
+ // 5(handled above)
// 4.4
// 4.4.0
// 4.4.x
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira added a comment. In http://reviews.llvm.org/D14727#290539, @jroelofs wrote: > testcase? Gladly, but where are the tests for GCCVersion located? http://reviews.llvm.org/D14727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira added a comment. In http://reviews.llvm.org/D14727#290661, @jroelofs wrote: > hHave a look at the svn-blame for this file, then find other commits which > changed this part of the file, and see where those commits added tests. Thanks, looks like 189212 has a way to do it. http://reviews.llvm.org/D14727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira updated this revision to Diff 40370.
thiagomacieira added a comment.
v3: updated to add unit testing
http://reviews.llvm.org/D14727
Files:
lib/Driver/ToolChains.cpp
test/Driver/Inputs/gcc_version_parsing5/bin/.keep
test/Driver/Inputs/gcc_version_parsing5/lib/gcc/i386-unknown-linux/4.9.2/crtbegin.o
test/Driver/Inputs/gcc_version_parsing5/lib/gcc/i386-unknown-linux/5/crtbegin.o
test/Driver/linux-ld.c
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1230,14 +1230,17 @@
GCCVersion GoodVersion = {VersionText.str(), -1, -1, -1, "", "", ""};
if (First.first.getAsInteger(10, GoodVersion.Major) || GoodVersion.Major < 0)
return BadVersion;
- GoodVersion.MajorStr = First.first.str();
+ if (First.second.empty())
+return GoodVersion;
+
if (Second.first.getAsInteger(10, GoodVersion.Minor) || GoodVersion.Minor <
0)
return BadVersion;
GoodVersion.MinorStr = Second.first.str();
// First look for a number prefix and parse that if present. Otherwise just
// stash the entire patch string in the suffix, and leave the number
// unspecified. This covers versions strings such as:
+ // 5(handled above)
// 4.4
// 4.4.0
// 4.4.x
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -388,6 +388,15 @@
// CHECK-GCC-VERSION4: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
// CHECK-GCC-VERSION4:
"{{.*}}/Inputs/gcc_version_parsing4/bin/../lib/gcc/i386-unknown-linux/4.7.99{{/|}}crtbegin.o"
// CHECK-GCC-VERSION4:
"-L{{.*}}/Inputs/gcc_version_parsing4/bin/../lib/gcc/i386-unknown-linux/4.7.99"
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=i386-unknown-linux -m32 \
+// RUN: -ccc-install-dir %S/Inputs/gcc_version_parsing5/bin \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-GCC-VERSION5 %s
+// CHECK-GCC-VERSION5: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-GCC-VERSION5:
"{{.*}}/Inputs/gcc_version_parsing5/bin/../lib/gcc/i386-unknown-linux/5{{/|}}crtbegin.o"
+// CHECK-GCC-VERSION5:
"-L{{.*}}/Inputs/gcc_version_parsing5/bin/../lib/gcc/i386-unknown-linux/5"
//
// Test a simulated installation of libc++ on Linux, both through sysroot and
// the installation path of Clang.
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -1230,14 +1230,17 @@
GCCVersion GoodVersion = {VersionText.str(), -1, -1, -1, "", "", ""};
if (First.first.getAsInteger(10, GoodVersion.Major) || GoodVersion.Major < 0)
return BadVersion;
- GoodVersion.MajorStr = First.first.str();
+ if (First.second.empty())
+return GoodVersion;
+
if (Second.first.getAsInteger(10, GoodVersion.Minor) || GoodVersion.Minor < 0)
return BadVersion;
GoodVersion.MinorStr = Second.first.str();
// First look for a number prefix and parse that if present. Otherwise just
// stash the entire patch string in the suffix, and leave the number
// unspecified. This covers versions strings such as:
+ // 5(handled above)
// 4.4
// 4.4.0
// 4.4.x
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -388,6 +388,15 @@
// CHECK-GCC-VERSION4: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
// CHECK-GCC-VERSION4: "{{.*}}/Inputs/gcc_version_parsing4/bin/../lib/gcc/i386-unknown-linux/4.7.99{{/|}}crtbegin.o"
// CHECK-GCC-VERSION4: "-L{{.*}}/Inputs/gcc_version_parsing4/bin/../lib/gcc/i386-unknown-linux/4.7.99"
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=i386-unknown-linux -m32 \
+// RUN: -ccc-install-dir %S/Inputs/gcc_version_parsing5/bin \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-GCC-VERSION5 %s
+// CHECK-GCC-VERSION5: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-GCC-VERSION5: "{{.*}}/Inputs/gcc_version_parsing5/bin/../lib/gcc/i386-unknown-linux/5{{/|}}crtbegin.o"
+// CHECK-GCC-VERSION5: "-L{{.*}}/Inputs/gcc_version_parsing5/bin/../lib/gcc/i386-unknown-linux/5"
//
// Test a simulated installation of libc++ on Linux, both through sysroot and
// the installation path of Clang.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14727: [Driver] Adapt Linux::GCCVersion::Parse to match GCC 5 installations
thiagomacieira added a comment. What happens now? http://reviews.llvm.org/D14727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
