[Clang] [Sema] Improve support for `__restrict`-qualified member functions #83855

Sirraide · 2024-03-04T15:34:16Z

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:

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 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:

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.~~ Update: It seems that my interpretation of this was just wrong: GCC handles __restrict in this position just like any other cv-qualifier on a function parameter—i.e. it only matters if it’s specified in the definition; the confusing part about this to me was that it’s syntactically in the same position as const and volatile, but that’s about the only thing it has in common with those two. So yes, this behaviour definitely makes sense then if you look at it that way.

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:

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 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:

// '__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.

Issues with other approaches

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:

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.
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.

`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*):

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):

// 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:

[*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:

[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):

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:

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:

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.
Stripping __restrict from the function type of a MemberPointerType whenever we construct one.
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.
Propagate __restrict properly in template instantiation.
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.

…tions in canonical type

Sirraide · 2024-03-04T15:36:45Z

Sorry for the rather humongous pr description, but I tried to handle every edge case I could come up with (as well as include my rationale as to why I’ve decided to handle them that way)—thought it’s still possible that I missed some.

Sirraide · 2024-03-04T15:39:51Z

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

+  // 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();
+      if (Restrict)
+        EPI.TypeQuals.addRestrict();
+      else
+        EPI.TypeQuals.removeRestrict();
+      MD->setType(Context.getFunctionType(FPT->getReturnType(),
+                                          FPT->getParamTypes(), EPI));
+    }
+  }
+


If there is a better place to put this, please let me know. This works, but I at least feel like we should be doing this somewhere earlier...

It seems to me that this should happen on the Function Declaration instantiation, not the defintion. Since this should propagate even if the function is not yet defined.

Actually, I think that’s not it. I got confused by this too just now: this here is about propagating __restrict-ness (or lack thereof since we may have to remove it again if the declaration is __restrict, but the definition isn’t, and we’re in GCC mode) to the definition of the function only.

Specifically, we can’t just make the declaration __restrict when we first create it; the declaration needs to be __restrict iff the template declaration is __restrict, and the definition needs to be __restrict iff the template defintion is __restrict; the former should happen automatically as part of template instantiation; this here is only about the latter.

I’m going to expand this comment a bit more to elaborate on this.

frederick-vs-ja · 2024-03-06T11:17:44Z

Reported 2 bugs for MSVC. (The first also affects standard cv-qualifiers.)

Sirraide · 2024-03-31T00:32:51Z

Update: it seems that the GCC behaviour is intended, and it actually makes a lot of sense now imo at least (see the initial pr comment for more info about this).

erichkeane

I did a review, but didn't stop to decide whether we SHOULD :) Aaron should be around 'sometime' to make that judgement, my restrict knowledge/reasoning is slim/none.

erichkeane · 2024-04-01T18:08:14Z

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

+  // 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();
+      if (Restrict)
+        EPI.TypeQuals.addRestrict();
+      else
+        EPI.TypeQuals.removeRestrict();
+      MD->setType(Context.getFunctionType(FPT->getReturnType(),
+                                          FPT->getParamTypes(), EPI));
+    }
+  }
+


It seems to me that this should happen on the Function Declaration instantiation, not the defintion. Since this should propagate even if the function is not yet defined.

Sirraide · 2024-05-20T17:09:14Z

ping

cor3ntin · 2025-05-09T17:36:55Z

@Sirraide, wait, did we all miss this PR? I am so sorry

Sirraide · 2025-05-09T18:13:21Z

@Sirraide, wait, did we all miss this PR? I am so sorry

This was a while ago, but I recall talking to @AaronBallman about this one several times, and I think our conclusion at some point was that it wasn’t worth supporting this divergent behaviour or sth like that but I could be wrong (we also had a discussion about in one of the Wednesday meetings iirc).

AaronBallman · 2025-05-12T11:38:47Z

@Sirraide, wait, did we all miss this PR? I am so sorry

This was a while ago, but I recall talking to @AaronBallman about this one several times, and I think our conclusion at some point was that it wasn’t worth supporting this divergent behaviour or sth like that but I could be wrong (we also had a discussion about in one of the Wednesday meetings iirc).

That matches my recollection. Use of __restrict on member functions is an outlier to begin with, there's divergence between compilers already, there's a reasonable chance of silent breaking changes by caring about __restrict where we didn't used to, so it seemed like we'd want more motivation before going down this path.

Sirraide added 12 commits March 1, 2024 13:56

[Clang] [Sema] Strip __restrict in cv-qualifier-list of member func…

eb5ebe3

…tions in canonical type

[Clang] Tests for new __restrict behaviour

68e633e

[Clang] Update __restrict tests

20d7a07

[Clang] WIP: Improved support for __restrict member functions

8f868c8

[Clang] Make restrict-this test more readable

215a53e

[Clang] Proper handling of __restrict 'this' in MSVCCompat mode

5068271

[Clang] Properly propagate __restrict in member function templates

7b5b682

[Clang] More tests for __restrict member functions

e9e0123

[Clang] More tests with function templates

18726cf

[Clang] Propagate __restrict properly in template instantiation

a5739dd

[NFC] clang-format

03dba07

[Clang] Actually check definition for __restrict

b6dc450

Sirraide added clang:frontend Language frontend issues, e.g. anything involving "Sema" extension:microsoft extension:gnu labels Mar 4, 2024

Sirraide requested review from AaronBallman and erichkeane March 4, 2024 15:34

Sirraide commented Mar 4, 2024

View reviewed changes

erichkeane reviewed Apr 1, 2024

View reviewed changes

Sirraide added 2 commits April 9, 2024 13:51

Merge branch 'main' into restrict

f66e6ff

[NFC] Expand comment a bit

2ddf900

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] [Sema] Improve support for `__restrict`-qualified member functions #83855

[Clang] [Sema] Improve support for `__restrict`-qualified member functions #83855

Uh oh!

Sirraide commented Mar 4, 2024 •

edited

Loading

Uh oh!

Sirraide commented Mar 4, 2024

Uh oh!

Sirraide Mar 4, 2024

Uh oh!

erichkeane Apr 1, 2024

Uh oh!

Sirraide Apr 9, 2024

Uh oh!

frederick-vs-ja commented Mar 6, 2024

Uh oh!

Sirraide commented Mar 31, 2024

Uh oh!

erichkeane left a comment

Uh oh!

erichkeane Apr 1, 2024

Uh oh!

Sirraide commented May 20, 2024

Uh oh!

cor3ntin commented May 9, 2025

Uh oh!

Sirraide commented May 9, 2025

Uh oh!

AaronBallman commented May 12, 2025

Uh oh!

Uh oh!

[Clang] [Sema] Improve support for __restrict-qualified member functions #83855

Are you sure you want to change the base?

[Clang] [Sema] Improve support for __restrict-qualified member functions #83855

Uh oh!

Conversation

Sirraide commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

__restrict in a declaration vs definition

__restrict member function pointers

this

Additional notes

Related issues wrt __restrict

Effects on existing behaviour

To-do list

Links

Uh oh!

Sirraide commented Mar 4, 2024

Uh oh!

Sirraide Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

erichkeane Apr 1, 2024

Choose a reason for hiding this comment

Uh oh!

Sirraide Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja commented Mar 6, 2024

Uh oh!

Sirraide commented Mar 31, 2024

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

erichkeane Apr 1, 2024

Choose a reason for hiding this comment

Uh oh!

Sirraide commented May 20, 2024

Uh oh!

cor3ntin commented May 9, 2025

Uh oh!

Sirraide commented May 9, 2025

Uh oh!

AaronBallman commented May 12, 2025

Uh oh!

Uh oh!

[Clang] [Sema] Improve support for `__restrict`-qualified member functions #83855

[Clang] [Sema] Improve support for `__restrict`-qualified member functions #83855

Sirraide commented Mar 4, 2024 •

edited

Loading

`__restrict` in a declaration vs definition

`__restrict` member function pointers

`this`

Related issues wrt `__restrict`