https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/83855
>From eb5ebe31657fc82fe3810beff8d4cfff2201bf54 Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Fri, 1 Mar 2024 13:56:32 +0100 Subject: [PATCH 01/13] [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 <aeternalm...@gmail.com> Date: Fri, 1 Mar 2024 15:05:17 +0100 Subject: [PATCH 02/13] [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@FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ@1@A" +// FIXME: `__restrict` needs to be included in the mangled name: +// FIXME-CHECK-DAG: @"?c@FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ@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@FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ@1@A" +// FIXME: See comment above. +// FIXME-CHECK-DAG: @"?f@FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ@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@FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ@1@A" +// FIXME: See comment above. +// FIXME-CHECK-DAG: @"?i@FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ@1@A" using J = int (); S<J> j; @@ -205,9 +208,11 @@ struct A { void foo() __restrict &&; }; void A::foo() __restrict & {} -// CHECK-DAG: @"?foo@A@PR19361@@QIGAEXXZ" +// FIXME: See comment above +// FIXME-CHECK-DAG: @"?foo@A@PR19361@@QIGAEXXZ" void A::foo() __restrict && {} -// CHECK-DAG: @"?foo@A@PR19361@@QIHAEXXZ" +// FIXME: See comment above +// FIXME-CHECK-DAG: @"?foo@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@V?$H@PAH@PR26029@@@PR26029@@QAE@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@S@CVRMemberFunctionQuals@@QEBAXXZ" +// X64-DAG: define dso_local void @"?b@S@CVRMemberFunctionQuals@@QECAXXZ" +// X64-DAG: define dso_local void @"?c@S@CVRMemberFunctionQuals@@QEIAAXXZ" +// X64-DAG: define dso_local void @"?d@S@CVRMemberFunctionQuals@@QEDAXXZ" +// X64-DAG: define dso_local void @"?e@S@CVRMemberFunctionQuals@@QEIBAXXZ" +// X64-DAG: define dso_local void @"?f@S@CVRMemberFunctionQuals@@QEICAXXZ" +// X64-DAG: define dso_local void @"?g@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@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 <aeternalm...@gmail.com> Date: Sat, 2 Mar 2024 09:34:52 +0100 Subject: [PATCH 03/13] [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@FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ@1@A" +// CHECK-DAG: @"?c@FTypeWithQuals@@3U?$S@$$A8@@IAAHXZ@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@FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ@1@A" +// CHECK-DAG: @"?f@FTypeWithQuals@@3U?$S@$$A8@@IGAAHXZ@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@FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ@1@A" +// CHECK-DAG: @"?i@FTypeWithQuals@@3U?$S@$$A8@@IHAAHXZ@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@A@PR19361@@QIGAEXXZ" +// CHECK-DAG: @"?foo@A@PR19361@@QIGAEXXZ" void A::foo() __restrict && {} -// FIXME: See comment above -// FIXME-CHECK-DAG: @"?foo@A@PR19361@@QIHAEXXZ" +// CHECK-DAG: @"?foo@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 <aeternalm...@gmail.com> Date: Sat, 2 Mar 2024 20:29:02 +0100 Subject: [PATCH 04/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 08:32:22 +0100 Subject: [PATCH 05/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 11:13:29 +0100 Subject: [PATCH 06/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 13:39:52 +0100 Subject: [PATCH 07/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 13:57:03 +0100 Subject: [PATCH 08/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 14:08:32 +0100 Subject: [PATCH 09/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 16:00:34 +0100 Subject: [PATCH 10/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 16:01:40 +0100 Subject: [PATCH 11/13] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 16:12:21 +0100 Subject: [PATCH 12/13] [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 { >From 2ddf9003e2d2f922af8bea16a41f4315768c80c3 Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Tue, 9 Apr 2024 14:35:20 +0200 Subject: [PATCH 13/13] [NFC] Expand comment a bit --- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 0e2ed20efe5741..4f1d12581d4a0f 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -5081,7 +5081,14 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, Function->setInnerLocStart(PatternDecl->getInnerLocStart()); Function->setRangeEnd(PatternDecl->getEndLoc()); - // Propagate '__restrict' properly. + // Propagate '__restrict' (or lack thereof) properly. + // + // Since '__restrict'-ness is allowed to differ between the declaration and + // definition of a function, we need to propagate the '__restrict'-ness of + // the template declaration to the instantiated declaration, and that of the + // template definition to the instantiated definition. The former happens + // automatically during template instantiation, so we only need to handle + // the latter here. if (auto MD = dyn_cast<CXXMethodDecl>(Function)) { bool Restrict = cast<CXXMethodDecl>(PatternDecl)->isEffectivelyRestrict(); if (Restrict != MD->getMethodQualifiers().hasRestrict()) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits