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

clang/include/clang/AST/ASTContext.h

@@ -1449,6 +1449,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,

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

clang/lib/AST/ASTContext.cpp

@@ -3541,9 +3541,8 @@ 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;
@@ -3572,6 +3571,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,

clang/lib/AST/DeclCXX.cpp

@@ -2537,27 +2537,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 {
@@ -2567,8 +2565,28 @@ 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.
+  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 {

clang/lib/CodeGen/CGDebugInfo.cpp

@@ -3323,9 +3323,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);
 }
 

clang/lib/Sema/SemaDecl.cpp

@@ -3974,6 +3974,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());
@@ -4040,6 +4041,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
@@ -4085,12 +4094,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)) {
@@ -4117,7 +4146,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

clang/lib/Sema/SemaExprCXX.cpp

@@ -1136,8 +1136,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
@@ -1180,7 +1180,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;
     }
   }
 
@@ -1219,7 +1222,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())

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -5081,6 +5081,28 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
   Function->setRangeEnd(PatternDecl->getEndLoc());
 
+  // 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()) {
+      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);
 

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 {}
+}

clang/test/SemaCXX/addr-of-overloaded-function.cpp

@@ -211,23 +211,15 @@ 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 {};
   };
 
 
   void QualifierTest() {
     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 {