[clang] [Clang] [Sema] Improve support for `__restrict`-qualified member functions (PR #83855)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 4 07:34:29 PST 2024
https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/83855
The exact semantics of `__restrict` when applied to member functions are somewhat nebulous seeing as it is a compiler extension, and as a result, there are quite a few edge cases that our implementation currently doesn’t handle very well (or at all). This pr is mainly about cases such as:
```c++
struct S {
void a() __restrict;
};
void S::a() { }
```
Both GCC and MSVC support this extension, and so does Clang; the problem is that our support for it is quite incomplete in some cases. Fortunately, GCC and MSVC agree on surprisingly many things here, but there are still enough places where they differ and which we’re not handling properly at the moment.
At the same time, it should be noted that [MSVC’s documentation](https://learn.microsoft.com/en-us/cpp/cpp/extension-restrict?view=msvc-170) surprisingly states that ‘The `__restrict` keyword is valid only on variables’, despite the fact that MSVC seems to support it on functions anyway (to the point where it affects how function names are mangled). The remainder of this discussion assumes that it is indeed supported and that the documentation is incomplete or incorrect.
## `__restrict` in a declaration vs definition
First, both GCC and MSVC allow a mismatch in `__restrict`-qualification between the declaration and definition of a member function:
```c++
struct S {
void a() __restrict;
void b();
};
// Both of these are accepted.
void S::a() { }
void S::b() __restrict { }
```
Unfortunately, that is where the similarities end; GCC only seems to care about `__restrict` being present on the definition; I’ve already opened a GCC bug report for that to ask for clarification whether this is actually intended.
MSVC does the opposite, to the point where the function name is mangled differently depending on whether `__restrict` is present on the *declaration* (`__restrict` on an out-of-line definition does not impact mangling at all and seems to be ignored entirely; see the section on `this` below). The fact that it has an effect on mangling seems to indicate that ignoring `__restrict` if it is only present on a function definition is intended, as you’d run into linker errors otherwise.
We currently do not support this at all. The best approach to fixing this currently seems to be simply ignoring `__restrict` when checking if member function types are compatible.
Also, to make talking about this a bit easier, let’s call a member function ‘effectively `__restrict`’,
- if both its declaration or definition contain `__restrict`; or
- in `MSVCCompat` mode, if its declaration contains `__restrict`; or
- otherwise, if its definition contains `__restrict`.
## `__restrict` member function pointers
This also has consequences on how `__restrict`-qualification is handled in pointers to member functions: both GCC and MSVC basically ignore `__restrict` in a pointer to member type entirely:
```c++
struct S {
void a() __restrict;
void b();
};
// '__restrict' is effectively ignored here.
static_assert(is_same_v<
void (S::*)() __restrict,
void (S::*)()
>);
static_assert(is_same_v<
Template<void (S::*)() __restrict>,
Template<void (S::*)() >
>);
// All of these are allowed.
void (S::*p1)() __restrict = &S::a;
void (S::*p2)() = &S::a;
void (S::*p3)() __restrict = &S::b;
void (S::*p4)() = &S::b;
```
At least with GCC, this behaviour seems intended, as [its documentation](https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Restricted-Pointers.html) explicitly states that:
> As with all outermost parameter qualifiers, `__restrict__` is ignored in function definition matching. This
means you only need to specify `__restrict__` in a function definition, rather than in a function prototype
as well.
Expectedly, `__is_same` also returns `true` in these cases in GCC (MSVC does not have `__is_same`). To support this behaviour, we should strip `__restrict` from the type of a pointer to member function; I’ve been talking this over with @AaronBallman for quite a bit, and we both agree that attempting to instead keep it and adjust all places where it might matter to ignore it seems infeasible.
However, we cannot simply strip `__restrict` from all function types because of another annoying detail that GCC and MSVC also both agree on: `__restrict` on regular function types *does* matter:
```c++
// '__restrict' is *not* ignored here.
static_assert(not is_same_v<
void () __restrict,
void ()
>);
static_assert(not is_same_v<
Template<void () __restrict>,
Template<void () >
>);
```
Fortunately, what seems to be working quite well is stripping `__restrict` from the function type whenever we create a `MemberPointerType`—this does mean we lose out on source fidelity a bit though—and ignoring `__restrict` checking if two function types are compatible for the purpose of merging function declarations.
<details>
<summary>Issues with other approaches</summary>
Aaron and I considered the possibility of *sometimes* stripping `__restrict` from the *canonical* type of a function type, but I ran into two issues with that:
1. We’d have to make this change in `getFunctionTypeInternal()`, which has no idea whether we’re about to use this thing in a `MemberPointerType`, so doing this would entail updating every last use of `getFunctionType()`, which would not only be a lot of work, but also very error-prone.
2. I have not actually checked this, but I believe the way we cache types makes this approach infeasible: Consider for instance some member function `void f() __restrict`, for which we then build the type `void () __restrict`; let’s call that type `(A)`; its canonical type, by this current approach, would be `void f()`. Now, what happens if we encounter, say `using X = void () __restrict`. This will try to construct `void () __restrict`, which is the same type as `(A)`! Since it already exists, we reuse `(A)`, but oh no, its canonical type doesn’t have `__restrict` even though it should in this case...
Another approach I considered but was quick to discard as well was *always* stripping `__restrict` from the function type of a `CXXMethodDecl` and tracking it as a bit in the `CXXMethodDecl` itself; however, every place that checks whether a `CXXMethodDecl` is `__restrict` would have to be updated as well, which is probably doable, but as Aaron also pointed out to me when I suggested that, we’d rather not have both an ‘is `__restrict`’ bit on the function type *as well as* an ‘is *actually* `__restrict`’ bit in `CXXMethodDecl` (or its type) if we can avoid it, because keeping those two in sync (or rather, remembering to use the latter as the former would basically never be set under that approach) sounds like it would only make things even more confusing than they already are.
In conclusion, stripping it when creating a `MemberPointerType` only and keeping it on the type of a `CXXMethodDecl` and just manually ignoring it in cases where other compilers ignore it seems to be the most feasible approach.
</details>
## `this`
Note that `__restrict` differs from `const` and `volatile` in that same position because, whereas the latter two apply to the object pointed to by `this`—i.e. `decltype(this)` would be `const S*` in a `const` member function of `S`—`__restrict`, on the other hand, is applied to the `this` pointer itself, i.e. `decltype(this)` is `S* __restrict` (*not* `__restrict S*`):
```c++
struct S {
void a() __restrict;
};
// Everyone agrees that this is `__restrict` and that `this` is `S* __restrict`.
void S::a() __restrict {
static_assert(is_same_v<
decltype(this),
S* __restrict
>);
}
```
We recently merged a pr (#83187) that fixed some bugs regarding this.
Both compilers also agree that `this` is `__restrict` iff the member function is effectively `__restrict`; the rest of the examples below apply to code inside an effectively `__restrict` member function.
Unfortunately, lambdas make this a bit more complicated: if `this` is *not* captured by value, then it still points to the same object, and it stands to reason that the captured `this` would still be `__restrict`. GCC agrees with this and so does MSVC (with a caveat, see below):
```c++
// GCC and MSVC both agree that `this` is `__restrict`.
[this]() {
static_assert(is_same<
decltype(this),
S* __restrict
>::value);
}
```
If we capture `this` by value, the situation is different; now, `this` refers to a different object, which means that this `this` is a different pointer, which would imply that we should no longer consider it `__restrict`; GCC agrees with that; however, MSVC still thinks it is `__restrict`:
```c++
[*this]() {
// Fails with GCC; passes with MSVC.
static_assert(is_same<
decltype(this),
S* __restrict
>::value);
};
```
I mentioned a caveat with MSVC, and you may have noticed that I switched to using `is_same<>::value` instead of `is_same_v<>` for these last two assertions; this is because, inside a lambda that captures `this` (by value or by reference), MSVC seems to disagree with itself whether or not `this` is `__restrict`:
```c++
[this]() {
is_same_v<decltype(this), S* __restrict>; // is false
is_same <decltype(this), S* __restrict>::value; // is true
};
```
If this is not an MSVC bug, then I don’t know what it is, candidly. The reason I said earlier that MSVC *does* seem to consider `this` to be `__restrict` (in both cases) is because attempting to instantiate an incomplete template gives this error (a failing `static_assert` unfortunately only prints ‘static assertion failed’ with no extra information with MSVC, necessitating this workaround):
```c++
template <typename>
struct Incomplete;
// Within a lambda that captures `this` or `*this`, MSVC gives:
// error C2079: 'foo' uses undefined struct 'Incomplete<S *__restrict >'
Incomplete<decltype(this)> foo;
```
Additionally, there is another MSVC issue I’ve run into: for the life of me, I cannot get `decltype(&S::a)` (where `S::a` is `__restrict`) to compare equal to any other type; the `Incomplete` trick above gives
`void (__cdecl S::* )(void)`, but even that does not work. I suspect this is another MSVC bug.
## Additional notes
Templates and explicit template specialisations behave just as though they were regular member functions wrt `__restrict` in both GCC and MSVC. Furthermore, the presence of other qualifiers (e.g. `const` or `&&`) does not seem to have any impact on the behaviour of `__restrict`.
This entire analysis is based on what does and does not compile, or what types are or aren’t equal. I have not taken into consideration how the presence of `__restrict` does or does not impact codegen in either GCC or MSVC, and I don’t believe there is a good reason to do that, seeing as this is mainly about ensuring that we agree with GCC/MSVC on what type something is.
That is, whether we actually do anything based on whether e.g. `__restrict` is only present on the declaration or definition is up to us, and we don’t necessarily need to agree with other compilers here. After all, it’s not like we’re ever just conjuring a `__restrict` out of thin air: if codegen ends up being different because we think a function is `__restrict`, then that means a user put `__restrict` *somewhere*, and if they didn’t want it to be `__restrict`, they shouldn’t have written `__restrict` *anywhere* in the first place. This pr is entirely about ensuring that code that compiles with GCC or MSVC compiles and does *something valid* when compiled with Clang.
## Related issues wrt `__restrict`
MSVC (but not GCC) allows constructors and destructors to be marked as `__restrict` (see #27469). I would argue we should at least support this in `MSVCCompat` mode, but I’m not opposed to also just supporting it in general (note that, although cv-qualifiers are not allowed on constructors/destructors, I would argue that that is because it just doesn’t make sense for an object that is being constructed/destroyed to be e.g. `const`; however, `__restrict` is different because it applies to `this`, and not `*this`, so there may be situations where there might be value in making it `__restrict`).
As an aside, since we’re already talking about `__restrict`, MSVC allows a `T*&` to bind to a `T* __restrict` (see #27383 and maybe #63662), i.e. the following code is accepted by MSVC:
```c++
int* __restrict x;
int*& y = x; // alternatively: int** y = &x;
```
GCC and Clang both reject this, and it seems semantically wrong to allow this (GCC and Clang also both complain about this in C, where `restrict` is actually part of the standard). There could be a case for supporting this in `MSVCCompat` mode (maybe downgraded to a warning as in C), but I’d leave it as is otherwise (and maybe close the issue(s) in question as wontfix in that case). This should also be a separate pr, if we do decide to change something here; I just wanted to bring this up because, if I’m already improving support for `__restrict`, I might as well look into other issues with it.
## Effects on existing behaviour
Most of the GCC-specific behaviour for `this` has already been implemented by #83187; I’ve refactored the code a bit and added support for MSVC-specific behaviour.
Currently, Clang does not support a mismatch in `__restrict`-qualification between the declaration and definition of a member function, which means that allowing that and implementing its semantics properly (‘properly’ as far as we can tell, anyway; all of this is awfully ill-documented...) shouldn’t cause any problems.
One thing to note is that stripping `__restrict` in some cases definitely impacts source fidelity; I have yet to look into possible solutions or workarounds for that, but also, I personally would argue that that is less important than getting the semantics of `__restrict` right in the first place.
## To-do list
Supporting GCC’s and MSVC’s semantics wrt `__restrict` seems to entail:
- [x] Allowing a mismatch between `__restrict`-qualification in the declaration and definition of a member function. The most straight-forward solution seems to be to ignore `__restrict` when merging member function declarations.
- [x] Stripping `__restrict` from the function type of a `MemberPointerType` whenever we construct one.
- [x] In the body of an effectively `__restrict` member function of some class `S`, make `this` always `__restrict`, unless we’re inside a lambda that captures `this` by value in non-`MSVCCompat` mode.
- [x] Propagate `__restrict` properly in template instantiation.
- [x] Add tests for all of this.
- [ ] Decide whether we want to allow `__restrict` constructors/destructors (possibly only in `MSVCCompat` mode).
- [ ] Address potential source fidelity concerns.
I’ve already implemented the points above; fortunately, I haven’t run into any major issues with the approach detailed above.
## Links
- Godbolt link to make sure the code examples here actually do what I say they do: https://godbolt.org/z/TxsMedxcY
- GCC bug I filed: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114210
- GCC docs: https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Restricted-Pointers.html
- MSVC docs: https://learn.microsoft.com/en-us/cpp/cpp/extension-restrict?view=msvc-170
This fixes #11039.
>From eb5ebe31657fc82fe3810beff8d4cfff2201bf54 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Fri, 1 Mar 2024 13:56:32 +0100
Subject: [PATCH 01/12] [Clang] [Sema] Strip `__restrict` in cv-qualifier-list
of member functions in canonical type
---
clang/lib/AST/ASTContext.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 5a8fae76a43a4d..91f1ccf429521f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4418,7 +4418,8 @@ QualType ASTContext::getFunctionTypeInternal(
// Determine whether the type being created is already canonical or not.
bool isCanonical = !Unique && IsCanonicalExceptionSpec &&
- isCanonicalResultType(ResultTy) && !EPI.HasTrailingReturn;
+ isCanonicalResultType(ResultTy) &&
+ !EPI.HasTrailingReturn && !EPI.TypeQuals.hasRestrict();
for (unsigned i = 0; i != NumArgs && isCanonical; ++i)
if (!ArgArray[i].isCanonicalAsParam())
isCanonical = false;
@@ -4439,6 +4440,7 @@ QualType ASTContext::getFunctionTypeInternal(
llvm::SmallVector<QualType, 8> ExceptionTypeStorage;
FunctionProtoType::ExtProtoInfo CanonicalEPI = EPI;
CanonicalEPI.HasTrailingReturn = false;
+ CanonicalEPI.TypeQuals.removeRestrict();
if (IsCanonicalExceptionSpec) {
// Exception spec is already OK.
>From 68e633ea0d26195b2aa4ced5f541497e8e5ae13a Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Fri, 1 Mar 2024 15:05:17 +0100
Subject: [PATCH 02/12] [Clang] Tests for new `__restrict` behaviour
---
clang/test/CodeGenCXX/mangle-ms-cxx11.cpp | 15 +++++---
clang/test/CodeGenCXX/mangle-ms.cpp | 36 +++++++++++++++++++
.../SemaCXX/addr-of-overloaded-function.cpp | 8 -----
.../restrict-member-function-matching.cpp | 21 +++++++++++
4 files changed, 67 insertions(+), 13 deletions(-)
create mode 100644 clang/test/SemaCXX/restrict-member-function-matching.cpp
diff --git a/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp b/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
index 312c70cc740eb3..4900c4ad272bc3 100644
--- a/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ b/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -16,7 +16,8 @@ S<B> b;
using C = int () __restrict;
S<C> c;
-// CHECK-DAG: @"?c at FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ at 1@A"
+// FIXME: `__restrict` needs to be included in the mangled name:
+// FIXME-CHECK-DAG: @"?c at FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ at 1@A"
using D = int () const &;
S<D> d;
@@ -28,7 +29,8 @@ S<E> e;
using F = int () __restrict &;
S<F> f;
-// CHECK-DAG: @"?f at FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ at 1@A"
+// FIXME: See comment above.
+// FIXME-CHECK-DAG: @"?f at FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ at 1@A"
using G = int () const &&;
S<G> g;
@@ -40,7 +42,8 @@ S<H> h;
using I = int () __restrict &&;
S<I> i;
-// CHECK-DAG: @"?i at FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ at 1@A"
+// FIXME: See comment above.
+// FIXME-CHECK-DAG: @"?i at FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ at 1@A"
using J = int ();
S<J> j;
@@ -205,9 +208,11 @@ struct A {
void foo() __restrict &&;
};
void A::foo() __restrict & {}
-// CHECK-DAG: @"?foo at A@PR19361@@QIGAEXXZ"
+// FIXME: See comment above
+// FIXME-CHECK-DAG: @"?foo at A@PR19361@@QIGAEXXZ"
void A::foo() __restrict && {}
-// CHECK-DAG: @"?foo at A@PR19361@@QIHAEXXZ"
+// FIXME: See comment above
+// FIXME-CHECK-DAG: @"?foo at A@PR19361@@QIHAEXXZ"
}
int operator"" _deg(long double) { return 0; }
diff --git a/clang/test/CodeGenCXX/mangle-ms.cpp b/clang/test/CodeGenCXX/mangle-ms.cpp
index cf69a83bbdf8c6..4cd28970f8094c 100644
--- a/clang/test/CodeGenCXX/mangle-ms.cpp
+++ b/clang/test/CodeGenCXX/mangle-ms.cpp
@@ -504,3 +504,39 @@ void runOnFunction() {
}
// CHECK-DAG: call {{.*}} @"??0?$L at V?$H at PAH@PR26029@@@PR26029@@QAE at XZ"
}
+
+namespace CVRMemberFunctionQuals {
+struct S {
+ void a() const;
+ void b() volatile;
+ void c() __restrict;
+ void d() const volatile;
+ void e() const __restrict;
+ void f() volatile __restrict;
+ void g() const volatile __restrict;
+
+ void h();
+};
+
+// X64-DAG: define dso_local void @"?a at S@CVRMemberFunctionQuals@@QEBAXXZ"
+// X64-DAG: define dso_local void @"?b at S@CVRMemberFunctionQuals@@QECAXXZ"
+// X64-DAG: define dso_local void @"?c at S@CVRMemberFunctionQuals@@QEIAAXXZ"
+// X64-DAG: define dso_local void @"?d at S@CVRMemberFunctionQuals@@QEDAXXZ"
+// X64-DAG: define dso_local void @"?e at S@CVRMemberFunctionQuals@@QEIBAXXZ"
+// X64-DAG: define dso_local void @"?f at S@CVRMemberFunctionQuals@@QEICAXXZ"
+// X64-DAG: define dso_local void @"?g at S@CVRMemberFunctionQuals@@QEIDAXXZ"
+void S::a() const {}
+void S::b() volatile {}
+void S::c() __restrict {}
+void S::d() const volatile {}
+void S::e() const __restrict {}
+void S::f() volatile __restrict {}
+void S::g() const volatile __restrict {}
+
+// MSVC allows a mismatch in `__restrict`-qualification between a function
+// declaration and definition and includes the qualifier in the ABI only if
+// it is present in the declaration.
+//
+// X64-DAG: define dso_local void @"?h at S@CVRMemberFunctionQuals@@QEAAXXZ"
+void S::h() __restrict {}
+}
diff --git a/clang/test/SemaCXX/addr-of-overloaded-function.cpp b/clang/test/SemaCXX/addr-of-overloaded-function.cpp
index dd1c3462c8c1f4..0d387ce1fb133d 100644
--- a/clang/test/SemaCXX/addr-of-overloaded-function.cpp
+++ b/clang/test/SemaCXX/addr-of-overloaded-function.cpp
@@ -211,11 +211,7 @@ namespace test1 {
void N() {};
void C() const {};
void V() volatile {};
- void R() __restrict {};
void CV() const volatile {};
- void CR() const __restrict {};
- void VR() volatile __restrict {};
- void CVR() const volatile __restrict {};
};
@@ -223,11 +219,7 @@ namespace test1 {
void (Qualifiers::*X)();
X = &Qualifiers::C; // expected-error-re {{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const': different qualifiers (unqualified vs 'const')}}
X = &Qualifiers::V; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile': different qualifiers (unqualified vs 'volatile')}}
- X = &Qualifiers::R; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers (unqualified vs '__restrict')}}
X = &Qualifiers::CV; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers (unqualified vs 'const volatile')}}
- X = &Qualifiers::CR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers (unqualified vs 'const __restrict')}}
- X = &Qualifiers::VR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers (unqualified vs 'volatile __restrict')}}
- X = &Qualifiers::CVR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile __restrict': different qualifiers (unqualified vs 'const volatile __restrict')}}
}
struct Dummy {
diff --git a/clang/test/SemaCXX/restrict-member-function-matching.cpp b/clang/test/SemaCXX/restrict-member-function-matching.cpp
new file mode 100644
index 00000000000000..e3e1edad718bc4
--- /dev/null
+++ b/clang/test/SemaCXX/restrict-member-function-matching.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+struct S{ void a(); };
+
+// GCC allows '__restrict' in the cv-qualifier-seq of member functions but
+// ignores it for pretty much everything (except that the type of 'this' in
+// is '__restrict' iff the *definition* is '__restrict' as well).
+static_assert(__is_same(void (S::*) (), void (S::*) () __restrict));
+
+namespace gh11039 {
+class foo {
+ int member[4];
+
+ void bar(int * a);
+};
+
+void foo::bar(int * a) __restrict {
+ member[3] = *a;
+}
+}
\ No newline at end of file
>From 20d7a072f4fe4b98d3af67f30c7cc56deb1cfdea Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sat, 2 Mar 2024 09:34:52 +0100
Subject: [PATCH 03/12] [Clang] Update `__restrict` tests
---
clang/test/CodeGenCXX/mangle-ms-cxx11.cpp | 15 ++++-------
.../restrict-member-function-matching.cpp | 26 ++++++++++++++++---
2 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp b/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
index 4900c4ad272bc3..312c70cc740eb3 100644
--- a/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ b/clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -16,8 +16,7 @@ S<B> b;
using C = int () __restrict;
S<C> c;
-// FIXME: `__restrict` needs to be included in the mangled name:
-// FIXME-CHECK-DAG: @"?c at FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ at 1@A"
+// CHECK-DAG: @"?c at FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ at 1@A"
using D = int () const &;
S<D> d;
@@ -29,8 +28,7 @@ S<E> e;
using F = int () __restrict &;
S<F> f;
-// FIXME: See comment above.
-// FIXME-CHECK-DAG: @"?f at FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ at 1@A"
+// CHECK-DAG: @"?f at FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ at 1@A"
using G = int () const &&;
S<G> g;
@@ -42,8 +40,7 @@ S<H> h;
using I = int () __restrict &&;
S<I> i;
-// FIXME: See comment above.
-// FIXME-CHECK-DAG: @"?i at FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ at 1@A"
+// CHECK-DAG: @"?i at FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ at 1@A"
using J = int ();
S<J> j;
@@ -208,11 +205,9 @@ struct A {
void foo() __restrict &&;
};
void A::foo() __restrict & {}
-// FIXME: See comment above
-// FIXME-CHECK-DAG: @"?foo at A@PR19361@@QIGAEXXZ"
+// CHECK-DAG: @"?foo at A@PR19361@@QIGAEXXZ"
void A::foo() __restrict && {}
-// FIXME: See comment above
-// FIXME-CHECK-DAG: @"?foo at A@PR19361@@QIHAEXXZ"
+// CHECK-DAG: @"?foo at A@PR19361@@QIHAEXXZ"
}
int operator"" _deg(long double) { return 0; }
diff --git a/clang/test/SemaCXX/restrict-member-function-matching.cpp b/clang/test/SemaCXX/restrict-member-function-matching.cpp
index e3e1edad718bc4..a538c9b55b603e 100644
--- a/clang/test/SemaCXX/restrict-member-function-matching.cpp
+++ b/clang/test/SemaCXX/restrict-member-function-matching.cpp
@@ -1,12 +1,32 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple=x86_64-pc-win32 -fms-compatibility %s
// expected-no-diagnostics
struct S{ void a(); };
-// GCC allows '__restrict' in the cv-qualifier-seq of member functions but
-// ignores it for pretty much everything (except that the type of 'this' in
-// is '__restrict' iff the *definition* is '__restrict' as well).
+template <typename A, typename B>
+struct is_same {
+ static constexpr bool value = false;
+};
+
+template <typename A>
+struct is_same<A, A> {
+ static constexpr bool value = true;
+};
+
+// GCC and MSVC allows '__restrict' in the cv-qualifier-seq of member functions
+// but ignore it for the purpose of matching and type comparisons.
static_assert(__is_same(void (S::*) (), void (S::*) () __restrict));
+static_assert(__is_same(void (S::*) () &, void (S::*) () __restrict &));
+static_assert(__is_same(void (S::*) () &&, void (S::*) () __restrict &&));
+static_assert(is_same<void (S::*) (), void (S::*) () __restrict>::value);
+static_assert(is_same<void (S::*) () &, void (S::*) () __restrict &>::value);
+static_assert(is_same<void (S::*) () &&, void (S::*) () __restrict &&>::value);
+
+// Non-member function types with '__restrict' are distinct types.
+using A = void () __restrict;
+using B = void ();
+static_assert(!is_same<A, B>::value);
namespace gh11039 {
class foo {
>From 8f868c8485e5e05df2437eca075d17eec208f8e4 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sat, 2 Mar 2024 20:29:02 +0100
Subject: [PATCH 04/12] [Clang] WIP: Improved support for `__restrict` member
functions
---
clang/include/clang/AST/ASTContext.h | 4 +
clang/lib/AST/ASTContext.cpp | 27 +++++--
clang/lib/Sema/SemaDecl.cpp | 43 ++++++++--
clang/lib/Sema/SemaDeclCXX.cpp | 20 +++++
.../restrict-member-function-matching.cpp | 38 +++++++--
clang/test/SemaCXX/restrict-this.cpp | 78 ++++++++++++++++++-
6 files changed, 191 insertions(+), 19 deletions(-)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index ff6b64c7f72d57..5b0a59be08519d 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1435,6 +1435,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
/// The class \p Cls is a \c Type because it could be a dependent name.
QualType getMemberPointerType(QualType T, const Type *Cls) const;
+private:
+ QualType getMemberPointerTypeInternal(QualType T, const Type *Cls) const;
+
+public:
/// Return a non-unique reference to the type for a variable array of
/// the specified element type.
QualType getVariableArrayType(QualType EltTy, Expr *NumElts,
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 91f1ccf429521f..abc5e3a9840d85 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3479,9 +3479,7 @@ QualType ASTContext::getRValueReferenceType(QualType T) const {
return QualType(New, 0);
}
-/// getMemberPointerType - Return the uniqued reference to the type for a
-/// member pointer to the specified type, in the specified class.
-QualType ASTContext::getMemberPointerType(QualType T, const Type *Cls) const {
+QualType ASTContext::getMemberPointerTypeInternal(QualType T, const Type *Cls) const {
// Unique pointers, to guarantee there is only one pointer of a particular
// structure.
llvm::FoldingSetNodeID ID;
@@ -3510,6 +3508,26 @@ QualType ASTContext::getMemberPointerType(QualType T, const Type *Cls) const {
return QualType(New, 0);
}
+/// getMemberPointerType - Return the uniqued reference to the type for a
+/// member pointer to the specified type, in the specified class.
+QualType ASTContext::getMemberPointerType(QualType T, const Type *Cls) const {
+ bool Paren = isa<ParenType>(T);
+ T = T.IgnoreParens();
+
+ // GCC and MSVC effectively drop '__restrict' from the function type,
+ // so do the same as well.
+ if (auto *FPT = dyn_cast<FunctionProtoType>(T);
+ FPT && FPT->getMethodQuals().hasRestrict()) {
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ EPI.TypeQuals.removeRestrict();
+ T = getFunctionType(FPT->getReturnType(), FPT->getParamTypes(), EPI);
+ }
+
+ if (Paren)
+ T = getParenType(T);
+ return getMemberPointerTypeInternal(T, Cls);
+}
+
/// getConstantArrayType - Return the unique reference to the type for an
/// array of the specified element type.
QualType ASTContext::getConstantArrayType(QualType EltTy,
@@ -4419,7 +4437,7 @@ QualType ASTContext::getFunctionTypeInternal(
// Determine whether the type being created is already canonical or not.
bool isCanonical = !Unique && IsCanonicalExceptionSpec &&
isCanonicalResultType(ResultTy) &&
- !EPI.HasTrailingReturn && !EPI.TypeQuals.hasRestrict();
+ !EPI.HasTrailingReturn;
for (unsigned i = 0; i != NumArgs && isCanonical; ++i)
if (!ArgArray[i].isCanonicalAsParam())
isCanonical = false;
@@ -4440,7 +4458,6 @@ QualType ASTContext::getFunctionTypeInternal(
llvm::SmallVector<QualType, 8> ExceptionTypeStorage;
FunctionProtoType::ExtProtoInfo CanonicalEPI = EPI;
CanonicalEPI.HasTrailingReturn = false;
- CanonicalEPI.TypeQuals.removeRestrict();
if (IsCanonicalExceptionSpec) {
// Exception spec is already OK.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 9fdd8eb236d1ee..c110e8a35d4ce0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3975,6 +3975,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
const CXXMethodDecl *OldMethod = dyn_cast<CXXMethodDecl>(Old);
CXXMethodDecl *NewMethod = dyn_cast<CXXMethodDecl>(New);
+ const FunctionDecl *RemoveRestrictFrom = nullptr; // See below.
if (OldMethod && NewMethod) {
// Preserve triviality.
NewMethod->setTrivial(OldMethod->isTrivial());
@@ -4041,6 +4042,14 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
<< getSpecialMember(OldMethod);
return true;
}
+
+ // GCC and MSVC allow a mismatch in '__restrict'-qualification between
+ // member function declarations, so remove it if it is present on one
+ // but not the other.
+ auto OldQuals = OldMethod->getMethodQualifiers();
+ auto NewQuals = NewMethod->getMethodQualifiers();
+ if (OldQuals.hasRestrict() != NewQuals.hasRestrict())
+ RemoveRestrictFrom = OldQuals.hasRestrict() ? Old : New;
}
// C++1z [over.load]p2
@@ -4086,12 +4095,32 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
// noreturn should now match unless the old type info didn't have it.
QualType OldQTypeForComparison = OldQType;
- if (!OldTypeInfo.getNoReturn() && NewTypeInfo.getNoReturn()) {
- auto *OldType = OldQType->castAs<FunctionProtoType>();
- const FunctionType *OldTypeForComparison
- = Context.adjustFunctionType(OldType, OldTypeInfo.withNoReturn(true));
- OldQTypeForComparison = QualType(OldTypeForComparison, 0);
- assert(OldQTypeForComparison.isCanonical());
+ QualType NewQTypeForComparison = NewQType;
+
+ // Helper to adjust the EPI of a function type.
+ auto AdjustFunctionType = [&](QualType QT, auto Adjust) -> QualType {
+ const auto *FPT = QT->castAs<FunctionProtoType>();
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ Adjust(EPI);
+ QualType Adjusted = Context.getFunctionType(FPT->getReturnType(),
+ FPT->getParamTypes(), EPI);
+ assert(Adjusted.isCanonical());
+ return Adjusted;
+ };
+
+ bool AddNoReturn = !OldTypeInfo.getNoReturn() && NewTypeInfo.getNoReturn();
+ if (AddNoReturn || RemoveRestrictFrom == Old) {
+ auto Adjust = [&](FunctionProtoType::ExtProtoInfo &EPI) {
+ EPI.ExtInfo = OldTypeInfo.withNoReturn(AddNoReturn);
+ if (RemoveRestrictFrom == Old)
+ EPI.TypeQuals.removeRestrict();
+ };
+ OldQTypeForComparison = AdjustFunctionType(OldQType, Adjust);
+ }
+
+ if (RemoveRestrictFrom == New) {
+ auto Adjust = [](auto &EPI) { EPI.TypeQuals.removeRestrict(); };
+ NewQTypeForComparison = AdjustFunctionType(NewQType, Adjust);
}
if (haveIncompatibleLanguageLinkages(Old, New)) {
@@ -4118,7 +4147,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
// CheckEquivalentExceptionSpec, and we don't want follow-on diagnostics
// about incompatible types under -fms-compatibility.
if (Context.hasSameFunctionTypeIgnoringExceptionSpec(OldQTypeForComparison,
- NewQType))
+ NewQTypeForComparison))
return MergeCompatibleFunctionDecls(New, Old, S, MergeTypeWithOld);
// If the types are imprecise (due to dependent constructs in friends or
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index d4e1dc67cb50a1..0dc4e133673abf 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -725,6 +725,26 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
Old->isDefined(OldDefinition, true))
CheckForFunctionRedefinition(New, OldDefinition);
+ // Both GCC and MSVC allow a mismatch in '__restrict'-qualification between
+ // the old and new declarations' cv-qualifier-seq's. Unfortunately, they
+ // also handle this mismatch differently: GCC considers '__restrict' on the
+ // definition to be authoritative (i.e. the type of 'this' is '__restrict'
+ // in the body of a definition, if that definition is '__restrict'-qualified),
+ // while MSVC does the opposite: it only looks at the first declaration).
+ //
+ // To support this behaviour, copy '__restrict' from the old declaration to
+ // the new one in MSVC mode only.
+ if (isa<CXXMethodDecl>(New) and getLangOpts().MSVCCompat) {
+ auto NewType = New->getType()->castAs<FunctionProtoType>();
+ auto Quals = Old->getType()->castAs<FunctionProtoType>()->getMethodQuals();
+ if (Quals.hasRestrict() && !NewType->getMethodQuals().hasRestrict()) {
+ FunctionProtoType::ExtProtoInfo EPI = NewType->getExtProtoInfo();
+ EPI.TypeQuals.addRestrict();
+ New->setType(Context.getFunctionType(NewType->getReturnType(),
+ NewType->getParamTypes(), EPI));
+ }
+ }
+
return Invalid;
}
diff --git a/clang/test/SemaCXX/restrict-member-function-matching.cpp b/clang/test/SemaCXX/restrict-member-function-matching.cpp
index a538c9b55b603e..5df5180f3d8e48 100644
--- a/clang/test/SemaCXX/restrict-member-function-matching.cpp
+++ b/clang/test/SemaCXX/restrict-member-function-matching.cpp
@@ -1,8 +1,11 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple=x86_64-pc-win32 -fms-compatibility %s
+// RUN: %clang_cc1 -fsyntax-only -verify -DMSVC=false %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple=x86_64-pc-win32 -fms-compatibility -DMSVC=true %s
// expected-no-diagnostics
-struct S{ void a(); };
+// GCC and MSVC allows '__restrict' in the cv-qualifier-seq of member functions
+// but ignore it for the purpose of matching and type comparisons. To match this
+// behaviour, we always strip restrict from pointers to member functions as well
+// as from member function declarations.
template <typename A, typename B>
struct is_same {
@@ -14,15 +17,38 @@ struct is_same<A, A> {
static constexpr bool value = true;
};
-// GCC and MSVC allows '__restrict' in the cv-qualifier-seq of member functions
-// but ignore it for the purpose of matching and type comparisons.
+struct S {
+ void a() __restrict;
+ void b() __restrict;
+ void c();
+};
+
+void S::a() __restrict { }
+void S::b() { }
+void S::c() __restrict { }
+
static_assert(__is_same(void (S::*) (), void (S::*) () __restrict));
static_assert(__is_same(void (S::*) () &, void (S::*) () __restrict &));
static_assert(__is_same(void (S::*) () &&, void (S::*) () __restrict &&));
+
static_assert(is_same<void (S::*) (), void (S::*) () __restrict>::value);
static_assert(is_same<void (S::*) () &, void (S::*) () __restrict &>::value);
static_assert(is_same<void (S::*) () &&, void (S::*) () __restrict &&>::value);
+static_assert(__is_same(decltype(&S::a), void (S::*) () __restrict));
+static_assert(__is_same(decltype(&S::a), void (S::*) ()));
+static_assert(__is_same(decltype(&S::b), void (S::*) () __restrict));
+static_assert(__is_same(decltype(&S::b), void (S::*) ()));
+static_assert(__is_same(decltype(&S::c), void (S::*) () __restrict));
+static_assert(__is_same(decltype(&S::c), void (S::*) ()));
+
+static_assert(is_same<decltype(&S::a), void (S::*) () __restrict>::value);
+static_assert(is_same<decltype(&S::a), void (S::*) ()>::value);
+static_assert(is_same<decltype(&S::b), void (S::*) () __restrict>::value);
+static_assert(is_same<decltype(&S::b), void (S::*) ()>::value);
+static_assert(is_same<decltype(&S::c), void (S::*) () __restrict>::value);
+static_assert(is_same<decltype(&S::c), void (S::*) ()>::value);
+
// Non-member function types with '__restrict' are distinct types.
using A = void () __restrict;
using B = void ();
@@ -38,4 +64,4 @@ class foo {
void foo::bar(int * a) __restrict {
member[3] = *a;
}
-}
\ No newline at end of file
+}
diff --git a/clang/test/SemaCXX/restrict-this.cpp b/clang/test/SemaCXX/restrict-this.cpp
index e78c8e0d56e2f8..f978458b620536 100644
--- a/clang/test/SemaCXX/restrict-this.cpp
+++ b/clang/test/SemaCXX/restrict-this.cpp
@@ -1,6 +1,13 @@
-// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -DMSVC=false %s
+// RUN: %clang_cc1 -verify -fsyntax-only -fms-compatibility -DMSVC=true %s
// expected-no-diagnostics
+// Check that '__restrict' is applied to 'this' as appropriate; note that a
+// mismatch in '__restrict'-qualification is allowed between the declaration
+// and definition of a member function. In MSVC mode, 'this' is '__restrict'
+// if the *declaration* is '__restrict'; otherwise, 'this' is '__restrict' if
+// the *definition* is '__restrict'.
+
struct C {
void f() __restrict {
static_assert(__is_same(decltype(this), C *__restrict));
@@ -14,6 +21,10 @@ struct C {
(void) [*this]() mutable { static_assert(__is_same(decltype(this), C *)); };
};
}
+
+ void a() __restrict;
+ void b() __restrict;
+ void c();
};
template <typename T> struct TC {
@@ -29,10 +40,75 @@ template <typename T> struct TC {
(void) [*this]() mutable { static_assert(__is_same(decltype(this), TC<int> *)); };
};
}
+
+ void a() __restrict;
+ void b() __restrict;
+ void c();
};
+// =========
+
+void C::a() __restrict {
+ static_assert(__is_same(decltype(this), C *__restrict));
+ (void) [this]() {
+ static_assert(__is_same(decltype(this), C *__restrict));
+ (void) [this]() { static_assert(__is_same(decltype(this), C *__restrict)); };
+ };
+}
+
+template <typename T>
+void TC<T>::a() __restrict {
+ static_assert(__is_same(decltype(this), TC<int> *__restrict));
+ (void) [this]() {
+ static_assert(__is_same(decltype(this), TC<int> *__restrict));
+ (void) [this]() { static_assert(__is_same(decltype(this), TC<int> *__restrict)); };
+ };
+}
+
+// =========
+
+void C::b() {
+ static_assert(__is_same(decltype(this), C *__restrict) == MSVC);
+ (void) [this]() {
+ static_assert(__is_same(decltype(this), C *__restrict) == MSVC);
+ (void) [this]() { static_assert(__is_same(decltype(this), C *__restrict) == MSVC); };
+ };
+}
+
+template <typename T>
+void TC<T>::b() {
+ static_assert(__is_same(decltype(this), TC<int> *__restrict) == MSVC);
+ (void) [this]() {
+ static_assert(__is_same(decltype(this), TC<int> *__restrict) == MSVC);
+ (void) [this]() { static_assert(__is_same(decltype(this), TC<int> *__restrict) == MSVC); };
+ };
+}
+
+// =========
+
+void C::c() __restrict {
+ static_assert(__is_same(decltype(this), C *__restrict) == !MSVC);
+ (void) [this]() {
+ static_assert(__is_same(decltype(this), C *__restrict) == !MSVC);
+ (void) [this]() { static_assert(__is_same(decltype(this), C *__restrict) == !MSVC); };
+ };
+}
+
+
+template <typename T>
+void TC<T>::c() __restrict {
+ static_assert(__is_same(decltype(this), TC<int> *__restrict) == !MSVC);
+ (void) [this]() {
+ static_assert(__is_same(decltype(this), TC<int> *__restrict) == !MSVC);
+ (void) [this]() { static_assert(__is_same(decltype(this), TC<int> *__restrict) == !MSVC); };
+ };
+}
+
void f() {
TC<int>{}.f();
+ TC<int>{}.a();
+ TC<int>{}.b();
+ TC<int>{}.c();
}
namespace gh18121 {
>From 215a53e59dc6e6384f6cb537974288d92fee67d5 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 08:32:22 +0100
Subject: [PATCH 05/12] [Clang] Make restrict-this test more readable
---
clang/test/SemaCXX/restrict-this.cpp | 68 ++++++++++++++++------------
1 file changed, 39 insertions(+), 29 deletions(-)
diff --git a/clang/test/SemaCXX/restrict-this.cpp b/clang/test/SemaCXX/restrict-this.cpp
index f978458b620536..0df86726e75231 100644
--- a/clang/test/SemaCXX/restrict-this.cpp
+++ b/clang/test/SemaCXX/restrict-this.cpp
@@ -8,17 +8,28 @@
// if the *declaration* is '__restrict'; otherwise, 'this' is '__restrict' if
// the *definition* is '__restrict'.
+#define Restrict(C) static_assert(__is_same(decltype(this), C* __restrict))
+#define NotRestrict(C) static_assert(__is_same(decltype(this), C*) || __is_same(decltype(this), const C*))
+
+#if MSVC
+# define RestrictIfMSVC Restrict
+# define RestrictIfGCC NotRestrict
+#else
+# define RestrictIfMSVC NotRestrict
+# define RestrictIfGCC Restrict
+#endif
+
struct C {
void f() __restrict {
- static_assert(__is_same(decltype(this), C *__restrict));
+ Restrict(C);
(void) [this]() {
- static_assert(__is_same(decltype(this), C *__restrict));
- (void) [this]() { static_assert(__is_same(decltype(this), C *__restrict)); };
+ Restrict(C);
+ (void) [this]() { Restrict(C); };
// By-value capture means 'this' is now a different object; do not
// make it __restrict.
- (void) [*this]() { static_assert(__is_same(decltype(this), const C *)); };
- (void) [*this]() mutable { static_assert(__is_same(decltype(this), C *)); };
+ (void) [*this]() { RestrictIfMSVC(C); };
+ (void) [*this]() mutable { RestrictIfMSVC(C); };
};
}
@@ -29,15 +40,15 @@ struct C {
template <typename T> struct TC {
void f() __restrict {
- static_assert(__is_same(decltype(this), TC<int> *__restrict));
+ Restrict(TC);
(void) [this]() {
- static_assert(__is_same(decltype(this), TC<int> *__restrict));
- (void) [this]() { static_assert(__is_same(decltype(this), TC<int> *__restrict)); };
+ Restrict(TC);
+ (void) [this]() { Restrict(TC); };
// By-value capture means 'this' is now a different object; do not
// make it __restrict.
- (void) [*this]() { static_assert(__is_same(decltype(this), const TC<int> *)); };
- (void) [*this]() mutable { static_assert(__is_same(decltype(this), TC<int> *)); };
+ (void) [*this]() { RestrictIfMSVC(TC); };
+ (void) [*this]() mutable { RestrictIfMSVC(TC); };
};
}
@@ -49,58 +60,57 @@ template <typename T> struct TC {
// =========
void C::a() __restrict {
- static_assert(__is_same(decltype(this), C *__restrict));
+ Restrict(C);
(void) [this]() {
- static_assert(__is_same(decltype(this), C *__restrict));
- (void) [this]() { static_assert(__is_same(decltype(this), C *__restrict)); };
+ Restrict(C);
+ (void) [*this]() { RestrictIfMSVC(C); };
};
}
template <typename T>
void TC<T>::a() __restrict {
- static_assert(__is_same(decltype(this), TC<int> *__restrict));
+ Restrict(TC);
(void) [this]() {
- static_assert(__is_same(decltype(this), TC<int> *__restrict));
- (void) [this]() { static_assert(__is_same(decltype(this), TC<int> *__restrict)); };
+ Restrict(TC);
+ (void) [*this]() { RestrictIfMSVC(TC); };
};
}
// =========
void C::b() {
- static_assert(__is_same(decltype(this), C *__restrict) == MSVC);
+ RestrictIfMSVC(C);
(void) [this]() {
- static_assert(__is_same(decltype(this), C *__restrict) == MSVC);
- (void) [this]() { static_assert(__is_same(decltype(this), C *__restrict) == MSVC); };
+ RestrictIfMSVC(C);
+ (void) [*this]() { RestrictIfMSVC(C); };
};
}
template <typename T>
void TC<T>::b() {
- static_assert(__is_same(decltype(this), TC<int> *__restrict) == MSVC);
+ RestrictIfMSVC(TC);
(void) [this]() {
- static_assert(__is_same(decltype(this), TC<int> *__restrict) == MSVC);
- (void) [this]() { static_assert(__is_same(decltype(this), TC<int> *__restrict) == MSVC); };
+ RestrictIfMSVC(TC);
+ (void) [*this]() { RestrictIfMSVC(TC); };
};
}
// =========
void C::c() __restrict {
- static_assert(__is_same(decltype(this), C *__restrict) == !MSVC);
+ RestrictIfGCC(C);
(void) [this]() {
- static_assert(__is_same(decltype(this), C *__restrict) == !MSVC);
- (void) [this]() { static_assert(__is_same(decltype(this), C *__restrict) == !MSVC); };
+ RestrictIfGCC(C);
+ (void) [*this]() { NotRestrict(C); };
};
}
-
template <typename T>
void TC<T>::c() __restrict {
- static_assert(__is_same(decltype(this), TC<int> *__restrict) == !MSVC);
+ RestrictIfGCC(TC);
(void) [this]() {
- static_assert(__is_same(decltype(this), TC<int> *__restrict) == !MSVC);
- (void) [this]() { static_assert(__is_same(decltype(this), TC<int> *__restrict) == !MSVC); };
+ RestrictIfGCC(TC);
+ (void) [*this]() { NotRestrict(TC); };
};
}
>From 5068271cde13d32e3bb80da852acbaa52f384527 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 11:13:29 +0100
Subject: [PATCH 06/12] [Clang] Proper handling of __restrict 'this' in
MSVCCompat mode
---
clang/include/clang/AST/DeclCXX.h | 11 +++++-
clang/lib/AST/DeclCXX.cpp | 56 ++++++++++++++++++----------
clang/lib/CodeGen/CGDebugInfo.cpp | 6 +--
clang/lib/Sema/SemaDeclCXX.cpp | 20 ----------
clang/lib/Sema/SemaExprCXX.cpp | 14 +++++--
clang/test/SemaCXX/restrict-this.cpp | 24 +++++++-----
6 files changed, 73 insertions(+), 58 deletions(-)
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 9cebaff63bb0db..11dac2a569ac4b 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -2208,13 +2208,20 @@ class CXXMethodDecl : public FunctionDecl {
return getNumParams() - (isExplicitObjectMemberFunction() ? 1 : 0);
}
- static QualType getThisType(const FunctionProtoType *FPT,
- const CXXRecordDecl *Decl);
+ /// Get the type of the \c this pointer for the given method type to be used
+ /// in a MemberPointerType or similar.
+ static QualType getThisTypeForMemberPtr(const FunctionProtoType *FPT,
+ const CXXRecordDecl *Decl);
Qualifiers getMethodQualifiers() const {
return getType()->castAs<FunctionProtoType>()->getMethodQuals();
}
+ /// Get whether this method should be considered '__restrict'-qualified,
+ /// irrespective of the presence or absence of '__restrict' on this
+ /// particular declaration.
+ bool isEffectivelyRestrict() const;
+
/// Retrieve the ref-qualifier associated with this method.
///
/// In the following example, \c f() has an lvalue ref-qualifier, \c g()
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index b4f2327d9c560a..106844919057df 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -2535,27 +2535,25 @@ CXXMethodDecl::overridden_methods() const {
static QualType getThisObjectType(ASTContext &C, const FunctionProtoType *FPT,
const CXXRecordDecl *Decl) {
+ // Unlike 'const' and 'volatile', a '__restrict' qualifier must be
+ // attached to the pointer type, not the pointee.
QualType ClassTy = C.getTypeDeclType(Decl);
- return C.getQualifiedType(ClassTy, FPT->getMethodQuals());
-}
-
-QualType CXXMethodDecl::getThisType(const FunctionProtoType *FPT,
- const CXXRecordDecl *Decl) {
+ Qualifiers Qs = FPT->getMethodQuals();
+ Qs.removeRestrict();
+ return C.getQualifiedType(ClassTy, Qs);
+}
+
+// This may be called in cases where we simply don't have a CXXMethodDecl,
+// in which case we don't have enough information to reason whether the type
+// of 'this' should include '__restrict', however, since we would strip
+// '__restrict' in MemberPointerTypes anyway, this ends up not being much of
+// an issue.
+QualType CXXMethodDecl::getThisTypeForMemberPtr(const FunctionProtoType *FPT,
+ const CXXRecordDecl *Decl) {
ASTContext &C = Decl->getASTContext();
QualType ObjectTy = ::getThisObjectType(C, FPT, Decl);
-
- // Unlike 'const' and 'volatile', a '__restrict' qualifier must be
- // attached to the pointer type, not the pointee.
- bool Restrict = FPT->getMethodQuals().hasRestrict();
- if (Restrict)
- ObjectTy.removeLocalRestrict();
-
- ObjectTy = C.getLangOpts().HLSL ? C.getLValueReferenceType(ObjectTy)
- : C.getPointerType(ObjectTy);
-
- if (Restrict)
- ObjectTy.addRestrict();
- return ObjectTy;
+ return C.getLangOpts().HLSL ? C.getLValueReferenceType(ObjectTy)
+ : C.getPointerType(ObjectTy);
}
QualType CXXMethodDecl::getThisType() const {
@@ -2565,8 +2563,26 @@ QualType CXXMethodDecl::getThisType() const {
// volatile X*, and if the member function is declared const volatile,
// the type of this is const volatile X*.
assert(isInstance() && "No 'this' for static methods!");
- return CXXMethodDecl::getThisType(getType()->castAs<FunctionProtoType>(),
- getParent());
+ ASTContext &C = getASTContext();
+ auto FPT = getType()->castAs<FunctionProtoType>();
+
+ QualType ObjectTy = ::getThisObjectType(C, FPT, getParent());
+ ObjectTy = C.getLangOpts().HLSL ? C.getLValueReferenceType(ObjectTy)
+ : C.getPointerType(ObjectTy);
+
+ if (isEffectivelyRestrict())
+ ObjectTy.addRestrict();
+ return ObjectTy;
+}
+
+bool CXXMethodDecl::isEffectivelyRestrict() const {
+ // MSVC only cares about '__restrict' on the first declaration; GCC only
+ // cares about the definition.
+ Qualifiers Qs =
+ getASTContext().getLangOpts().MSVCCompat
+ ? cast<CXXMethodDecl>(getFirstDecl())->getMethodQualifiers()
+ : getMethodQualifiers();
+ return Qs.hasRestrict();
}
QualType CXXMethodDecl::getFunctionObjectParameterReferenceType() const {
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index c2c01439f2dc99..97cef4756c22e5 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3322,9 +3322,9 @@ llvm::DIType *CGDebugInfo::CreateType(const MemberPointerType *Ty,
const FunctionProtoType *FPT =
Ty->getPointeeType()->castAs<FunctionProtoType>();
return DBuilder.createMemberPointerType(
- getOrCreateInstanceMethodType(
- CXXMethodDecl::getThisType(FPT, Ty->getMostRecentCXXRecordDecl()),
- FPT, U),
+ getOrCreateInstanceMethodType(CXXMethodDecl::getThisTypeForMemberPtr(
+ FPT, Ty->getMostRecentCXXRecordDecl()),
+ FPT, U),
ClassType, Size, /*Align=*/0, Flags);
}
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 0dc4e133673abf..d4e1dc67cb50a1 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -725,26 +725,6 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
Old->isDefined(OldDefinition, true))
CheckForFunctionRedefinition(New, OldDefinition);
- // Both GCC and MSVC allow a mismatch in '__restrict'-qualification between
- // the old and new declarations' cv-qualifier-seq's. Unfortunately, they
- // also handle this mismatch differently: GCC considers '__restrict' on the
- // definition to be authoritative (i.e. the type of 'this' is '__restrict'
- // in the body of a definition, if that definition is '__restrict'-qualified),
- // while MSVC does the opposite: it only looks at the first declaration).
- //
- // To support this behaviour, copy '__restrict' from the old declaration to
- // the new one in MSVC mode only.
- if (isa<CXXMethodDecl>(New) and getLangOpts().MSVCCompat) {
- auto NewType = New->getType()->castAs<FunctionProtoType>();
- auto Quals = Old->getType()->castAs<FunctionProtoType>()->getMethodQuals();
- if (Quals.hasRestrict() && !NewType->getMethodQuals().hasRestrict()) {
- FunctionProtoType::ExtProtoInfo EPI = NewType->getExtProtoInfo();
- EPI.TypeQuals.addRestrict();
- New->setType(Context.getFunctionType(NewType->getReturnType(),
- NewType->getParamTypes(), EPI));
- }
- }
-
return Invalid;
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c4750ce78fa9c1..5c3c1be604c1c3 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1130,8 +1130,8 @@ static QualType adjustCVQualifiersForCXXThisWithinLambda(
// member function. We then start with the innermost lambda and iterate
// outward checking to see if any lambda performs a by-copy capture of '*this'
// - and if so, any nested lambda must respect the 'constness' of that
- // capturing lamdbda's call operator.
- //
+ // capturing lamdbda's call operator. In MSVCCompat mode, we also consider
+ // 'this' to be '__restrict' even if it is captured by copy.
// Since the FunctionScopeInfo stack is representative of the lexical
// nesting of the lambda expressions during initial parsing (and is the best
@@ -1174,7 +1174,10 @@ static QualType adjustCVQualifiersForCXXThisWithinLambda(
if (C.isCopyCapture()) {
if (CurLSI->lambdaCaptureShouldBeConst())
ClassType.addConst();
- return ASTCtx.getPointerType(ClassType);
+ auto Ptr = ASTCtx.getPointerType(ClassType);
+ if (ThisTy.isRestrictQualified() && ASTCtx.getLangOpts().MSVCCompat)
+ Ptr.addRestrict();
+ return Ptr;
}
}
@@ -1213,7 +1216,10 @@ static QualType adjustCVQualifiersForCXXThisWithinLambda(
if (IsByCopyCapture) {
if (IsConstCapture)
ClassType.addConst();
- return ASTCtx.getPointerType(ClassType);
+ auto Ptr = ASTCtx.getPointerType(ClassType);
+ if (ThisTy.isRestrictQualified() && ASTCtx.getLangOpts().MSVCCompat)
+ Ptr.addRestrict();
+ return Ptr;
}
Closure = isLambdaCallOperator(Closure->getParent())
? cast<CXXRecordDecl>(Closure->getParent()->getParent())
diff --git a/clang/test/SemaCXX/restrict-this.cpp b/clang/test/SemaCXX/restrict-this.cpp
index 0df86726e75231..e4b1e31bbd87e6 100644
--- a/clang/test/SemaCXX/restrict-this.cpp
+++ b/clang/test/SemaCXX/restrict-this.cpp
@@ -9,7 +9,7 @@
// the *definition* is '__restrict'.
#define Restrict(C) static_assert(__is_same(decltype(this), C* __restrict))
-#define NotRestrict(C) static_assert(__is_same(decltype(this), C*) || __is_same(decltype(this), const C*))
+#define NotRestrict(C) static_assert(__is_same(decltype(this), C*))
#if MSVC
# define RestrictIfMSVC Restrict
@@ -28,7 +28,7 @@ struct C {
// By-value capture means 'this' is now a different object; do not
// make it __restrict.
- (void) [*this]() { RestrictIfMSVC(C); };
+ (void) [*this]() { RestrictIfMSVC(const C); };
(void) [*this]() mutable { RestrictIfMSVC(C); };
};
}
@@ -47,7 +47,7 @@ template <typename T> struct TC {
// By-value capture means 'this' is now a different object; do not
// make it __restrict.
- (void) [*this]() { RestrictIfMSVC(TC); };
+ (void) [*this]() { RestrictIfMSVC(const TC); };
(void) [*this]() mutable { RestrictIfMSVC(TC); };
};
}
@@ -63,7 +63,8 @@ void C::a() __restrict {
Restrict(C);
(void) [this]() {
Restrict(C);
- (void) [*this]() { RestrictIfMSVC(C); };
+ (void) [*this]() { RestrictIfMSVC(const C); };
+ (void) [*this]() mutable { RestrictIfMSVC(C); };
};
}
@@ -72,7 +73,8 @@ void TC<T>::a() __restrict {
Restrict(TC);
(void) [this]() {
Restrict(TC);
- (void) [*this]() { RestrictIfMSVC(TC); };
+ (void) [*this]() { RestrictIfMSVC(const TC); };
+ (void) [*this]() mutable { RestrictIfMSVC(TC); };
};
}
@@ -82,7 +84,8 @@ void C::b() {
RestrictIfMSVC(C);
(void) [this]() {
RestrictIfMSVC(C);
- (void) [*this]() { RestrictIfMSVC(C); };
+ (void) [*this]() { RestrictIfMSVC(const C); };
+ (void) [*this]() mutable { RestrictIfMSVC(C); };
};
}
@@ -91,7 +94,8 @@ void TC<T>::b() {
RestrictIfMSVC(TC);
(void) [this]() {
RestrictIfMSVC(TC);
- (void) [*this]() { RestrictIfMSVC(TC); };
+ (void) [*this]() { RestrictIfMSVC(const TC); };
+ (void) [*this]() mutable { RestrictIfMSVC(TC); };
};
}
@@ -101,7 +105,8 @@ void C::c() __restrict {
RestrictIfGCC(C);
(void) [this]() {
RestrictIfGCC(C);
- (void) [*this]() { NotRestrict(C); };
+ (void) [*this]() { NotRestrict(const C); };
+ (void) [*this]() mutable { NotRestrict(C); };
};
}
@@ -110,7 +115,8 @@ void TC<T>::c() __restrict {
RestrictIfGCC(TC);
(void) [this]() {
RestrictIfGCC(TC);
- (void) [*this]() { NotRestrict(TC); };
+ (void) [*this]() { NotRestrict(const TC); };
+ (void) [*this]() mutable { NotRestrict(TC); };
};
}
>From 7b5b6828f41853d3a3ebb9072c164a749f6e6012 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 13:39:52 +0100
Subject: [PATCH 07/12] [Clang] Properly propagate __restrict in member
function templates
---
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 9c696e072ba4a7..7e23610b6349e9 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5052,6 +5052,25 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
Function->setInnerLocStart(PatternDecl->getInnerLocStart());
Function->setRangeEnd(PatternDecl->getEndLoc());
+ // In non-MSVCCompat mode, we only care about the presence of a
+ // '__restrict' qualifier in the cv-qualifier-seq of a member
+ // function, and not its declaration.
+ if (auto MD = dyn_cast<CXXMethodDecl>(Function);
+ MD && !getLangOpts().MSVCCompat) {
+ bool Restrict =
+ cast<CXXMethodDecl>(PatternDecl)->getMethodQualifiers().hasRestrict();
+ if (Restrict != MD->getMethodQualifiers().hasRestrict()) {
+ const auto *FPT = MD->getType()->getAs<FunctionProtoType>();
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ if (Restrict)
+ EPI.TypeQuals.addRestrict();
+ else
+ EPI.TypeQuals.removeRestrict();
+ MD->setType(Context.getFunctionType(FPT->getReturnType(),
+ FPT->getParamTypes(), EPI));
+ }
+ }
+
EnterExpressionEvaluationContext EvalContext(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
>From e9e0123fadc825f7a34a7fe111522d1b15d2e422 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 13:57:03 +0100
Subject: [PATCH 08/12] [Clang] More tests for __restrict member functions
---
.../restrict-member-function-matching.cpp | 49 +++++++++++++------
1 file changed, 35 insertions(+), 14 deletions(-)
diff --git a/clang/test/SemaCXX/restrict-member-function-matching.cpp b/clang/test/SemaCXX/restrict-member-function-matching.cpp
index 5df5180f3d8e48..a97613554aa163 100644
--- a/clang/test/SemaCXX/restrict-member-function-matching.cpp
+++ b/clang/test/SemaCXX/restrict-member-function-matching.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -DMSVC=false %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple=x86_64-pc-win32 -fms-compatibility -DMSVC=true %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple=x86_64-pc-win32 -fms-compatibility %s
// expected-no-diagnostics
// GCC and MSVC allows '__restrict' in the cv-qualifier-seq of member functions
@@ -27,13 +27,14 @@ void S::a() __restrict { }
void S::b() { }
void S::c() __restrict { }
-static_assert(__is_same(void (S::*) (), void (S::*) () __restrict));
-static_assert(__is_same(void (S::*) () &, void (S::*) () __restrict &));
-static_assert(__is_same(void (S::*) () &&, void (S::*) () __restrict &&));
+void (S::*p1)() __restrict = &S::a;
+void (S::*p2)() = &S::a;
+void (S::*p3)() __restrict = &S::c;
+void (S::*p4)() = &S::c;
-static_assert(is_same<void (S::*) (), void (S::*) () __restrict>::value);
-static_assert(is_same<void (S::*) () &, void (S::*) () __restrict &>::value);
-static_assert(is_same<void (S::*) () &&, void (S::*) () __restrict &&>::value);
+static_assert(__is_same(void (S::*) (), void (S::*) () __restrict));
+static_assert(__is_same(void (S::*) () const &, void (S::*) () __restrict const &));
+static_assert(__is_same(void (S::*) () volatile &&, void (S::*) () volatile __restrict &&));
static_assert(__is_same(decltype(&S::a), void (S::*) () __restrict));
static_assert(__is_same(decltype(&S::a), void (S::*) ()));
@@ -42,12 +43,32 @@ static_assert(__is_same(decltype(&S::b), void (S::*) ()));
static_assert(__is_same(decltype(&S::c), void (S::*) () __restrict));
static_assert(__is_same(decltype(&S::c), void (S::*) ()));
-static_assert(is_same<decltype(&S::a), void (S::*) () __restrict>::value);
-static_assert(is_same<decltype(&S::a), void (S::*) ()>::value);
-static_assert(is_same<decltype(&S::b), void (S::*) () __restrict>::value);
-static_assert(is_same<decltype(&S::b), void (S::*) ()>::value);
-static_assert(is_same<decltype(&S::c), void (S::*) () __restrict>::value);
-static_assert(is_same<decltype(&S::c), void (S::*) ()>::value);
+template <typename>
+struct TS {
+ void a() __restrict;
+ void b() __restrict;
+ void c();
+};
+
+template <typename T>
+void TS<T>::b() __restrict { }
+
+template <typename T>
+void TS<T>::c() { }
+
+void (TS<int>::*p5)() __restrict = &TS<int>::a;
+void (TS<int>::*p6)() = &TS<int>::a;
+void (TS<int>::*p7)() __restrict = &TS<int>::c;
+void (TS<int>::*p8)() = &TS<int>::c;
+
+void h() {
+ TS<int>().a();
+ TS<int>().b();
+ TS<int>().c();
+ TS<double>().a();
+ TS<double>().b();
+ TS<double>().c();
+}
// Non-member function types with '__restrict' are distinct types.
using A = void () __restrict;
>From 18726cf9f7273be82e570cbde6120d56d0f9473a Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 14:08:32 +0100
Subject: [PATCH 09/12] [Clang] More tests with function templates
---
clang/test/SemaCXX/restrict-this.cpp | 83 ++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/clang/test/SemaCXX/restrict-this.cpp b/clang/test/SemaCXX/restrict-this.cpp
index e4b1e31bbd87e6..186035edc0eeaa 100644
--- a/clang/test/SemaCXX/restrict-this.cpp
+++ b/clang/test/SemaCXX/restrict-this.cpp
@@ -36,6 +36,15 @@ struct C {
void a() __restrict;
void b() __restrict;
void c();
+
+ template <typename U>
+ void ta() __restrict;
+
+ template <typename U>
+ void tb() __restrict;
+
+ template <typename U>
+ void tc();
};
template <typename T> struct TC {
@@ -55,6 +64,15 @@ template <typename T> struct TC {
void a() __restrict;
void b() __restrict;
void c();
+
+ template <typename U>
+ void ta() __restrict;
+
+ template <typename U>
+ void tb() __restrict;
+
+ template <typename U>
+ void tc();
};
// =========
@@ -78,6 +96,27 @@ void TC<T>::a() __restrict {
};
}
+template <typename T>
+void C::ta() __restrict {
+ Restrict(C);
+ (void) [this]() {
+ Restrict(C);
+ (void) [*this]() { RestrictIfMSVC(const C); };
+ (void) [*this]() mutable { RestrictIfMSVC(C); };
+ };
+}
+
+template <typename T>
+template <typename U>
+void TC<T>::ta() __restrict {
+ Restrict(TC);
+ (void) [this]() {
+ Restrict(TC);
+ (void) [*this]() { RestrictIfMSVC(const TC); };
+ (void) [*this]() mutable { RestrictIfMSVC(TC); };
+ };
+}
+
// =========
void C::b() {
@@ -99,6 +138,27 @@ void TC<T>::b() {
};
}
+template <typename T>
+void C::tb() {
+ RestrictIfMSVC(C);
+ (void) [this]() {
+ RestrictIfMSVC(C);
+ (void) [*this]() { RestrictIfMSVC(const C); };
+ (void) [*this]() mutable { RestrictIfMSVC(C); };
+ };
+}
+
+template <typename T>
+template <typename U>
+void TC<T>::tb() {
+ RestrictIfMSVC(TC);
+ (void) [this]() {
+ RestrictIfMSVC(TC);
+ (void) [*this]() { RestrictIfMSVC(const TC); };
+ (void) [*this]() mutable { RestrictIfMSVC(TC); };
+ };
+}
+
// =========
void C::c() __restrict {
@@ -120,6 +180,29 @@ void TC<T>::c() __restrict {
};
}
+template <typename T>
+void C::tc() __restrict {
+ RestrictIfGCC(C);
+ (void) [this]() {
+ RestrictIfGCC(C);
+ (void) [*this]() { NotRestrict(const C); };
+ (void) [*this]() mutable { NotRestrict(C); };
+ };
+}
+
+template <typename T>
+template <typename U>
+void TC<T>::tc() __restrict {
+ RestrictIfGCC(TC);
+ (void) [this]() {
+ RestrictIfGCC(TC);
+ (void) [*this]() { NotRestrict(const TC); };
+ (void) [*this]() mutable { NotRestrict(TC); };
+ };
+}
+
+// =========
+
void f() {
TC<int>{}.f();
TC<int>{}.a();
>From a5739dd698cc64e45451e308456308e513ba558e Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 16:00:34 +0100
Subject: [PATCH 10/12] [Clang] Propagate __restrict properly in template
instantiation
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 10 +++-------
clang/test/SemaCXX/restrict-this.cpp | 6 ++++++
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 7e23610b6349e9..7353faecdcdd8f 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5052,13 +5052,9 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
Function->setInnerLocStart(PatternDecl->getInnerLocStart());
Function->setRangeEnd(PatternDecl->getEndLoc());
- // In non-MSVCCompat mode, we only care about the presence of a
- // '__restrict' qualifier in the cv-qualifier-seq of a member
- // function, and not its declaration.
- if (auto MD = dyn_cast<CXXMethodDecl>(Function);
- MD && !getLangOpts().MSVCCompat) {
- bool Restrict =
- cast<CXXMethodDecl>(PatternDecl)->getMethodQualifiers().hasRestrict();
+ // Propagate '__restrict' properly.
+ if (auto MD = dyn_cast<CXXMethodDecl>(Function)) {
+ bool Restrict = cast<CXXMethodDecl>(PatternDecl)->isEffectivelyRestrict();
if (Restrict != MD->getMethodQualifiers().hasRestrict()) {
const auto *FPT = MD->getType()->getAs<FunctionProtoType>();
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
diff --git a/clang/test/SemaCXX/restrict-this.cpp b/clang/test/SemaCXX/restrict-this.cpp
index 186035edc0eeaa..3575ea2a3f561f 100644
--- a/clang/test/SemaCXX/restrict-this.cpp
+++ b/clang/test/SemaCXX/restrict-this.cpp
@@ -204,10 +204,16 @@ void TC<T>::tc() __restrict {
// =========
void f() {
+ C{}.ta<int>();
+ C{}.tb<int>();
+ C{}.tc<int>();
TC<int>{}.f();
TC<int>{}.a();
TC<int>{}.b();
TC<int>{}.c();
+ TC<int>{}.ta<int>();
+ TC<int>{}.tb<int>();
+ TC<int>{}.tc<int>();
}
namespace gh18121 {
>From 03dba07fbead6ee010d4a7e02f6a713d5d0bbb1d Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 16:01:40 +0100
Subject: [PATCH 11/12] [NFC] clang-format
---
clang/lib/AST/ASTContext.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index abc5e3a9840d85..a03fa353673858 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3479,7 +3479,8 @@ QualType ASTContext::getRValueReferenceType(QualType T) const {
return QualType(New, 0);
}
-QualType ASTContext::getMemberPointerTypeInternal(QualType T, const Type *Cls) const {
+QualType ASTContext::getMemberPointerTypeInternal(QualType T,
+ const Type *Cls) const {
// Unique pointers, to guarantee there is only one pointer of a particular
// structure.
llvm::FoldingSetNodeID ID;
@@ -4436,8 +4437,7 @@ QualType ASTContext::getFunctionTypeInternal(
// Determine whether the type being created is already canonical or not.
bool isCanonical = !Unique && IsCanonicalExceptionSpec &&
- isCanonicalResultType(ResultTy) &&
- !EPI.HasTrailingReturn;
+ isCanonicalResultType(ResultTy) && !EPI.HasTrailingReturn;
for (unsigned i = 0; i != NumArgs && isCanonical; ++i)
if (!ArgArray[i].isCanonicalAsParam())
isCanonical = false;
>From b6dc450ff1dcf312b897d218136d08155dcda5d5 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 16:12:21 +0100
Subject: [PATCH 12/12] [Clang] Actually check definition for __restrict
---
clang/lib/AST/DeclCXX.cpp | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 106844919057df..34968fd1c1181c 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -2578,11 +2578,13 @@ QualType CXXMethodDecl::getThisType() const {
bool CXXMethodDecl::isEffectivelyRestrict() const {
// MSVC only cares about '__restrict' on the first declaration; GCC only
// cares about the definition.
- Qualifiers Qs =
- getASTContext().getLangOpts().MSVCCompat
- ? cast<CXXMethodDecl>(getFirstDecl())->getMethodQualifiers()
- : getMethodQualifiers();
- return Qs.hasRestrict();
+ if (getASTContext().getLangOpts().MSVCCompat) {
+ Qualifiers Qs = cast<CXXMethodDecl>(getFirstDecl())->getMethodQualifiers();
+ return Qs.hasRestrict();
+ }
+
+ const auto *D = getDefinition();
+ return D && cast<CXXMethodDecl>(D)->getMethodQualifiers().hasRestrict();
}
QualType CXXMethodDecl::getFunctionObjectParameterReferenceType() const {
More information about the cfe-commits
mailing list