[clang] [Clang] Disallow explicit object parameters in more contexts (PR #89078)

via cfe-commits cfe-commits at lists.llvm.org
Tue May 28 11:09:08 PDT 2024


https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/89078

>From c611122688657287e8285edd9a2875e4975d26dd Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 17 Apr 2024 16:15:39 +0200
Subject: [PATCH 01/10] [Clang] Disallow explicit object parameters in more
 contexts

---
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 ++
 clang/lib/Sema/SemaDeclCXX.cpp                |  6 ++--
 clang/lib/Sema/SemaType.cpp                   | 30 +++++++++++++++++++
 clang/test/SemaCXX/cxx2b-deducing-this.cpp    | 22 ++++++++++++--
 4 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5ec0218aedfe8..680638d2e59c6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7494,6 +7494,8 @@ def err_explicit_object_parameter_mutable: Error<
 def err_invalid_explicit_object_type_in_lambda: Error<
   "invalid explicit object parameter type %0 in lambda with capture; "
   "the type must be the same as, or derived from, the lambda">;
+def err_explicit_object_parameter_invalid: Error<
+  "an explicit object parameter is not allowed here">;
 
 def err_ref_qualifier_overload : Error<
   "cannot overload a member function %select{without a ref-qualifier|with "
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 7669171fea56f..ea634df48e5de 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -11469,10 +11469,8 @@ void Sema::CheckExplicitObjectMemberFunction(Declarator &D,
     return;
 
   if (!DC || !DC->isRecord()) {
-    Diag(ExplicitObjectParam->getLocation(),
-         diag::err_explicit_object_parameter_nonmember)
-        << D.getSourceRange() << /*non-member=*/2 << IsLambda;
-    D.setInvalidType();
+    assert(D.isInvalidType() && "Explicit object parameter in non-member "
+                                "should have been diagnosed already");
     return;
   }
 
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 404c4e8e31b55..406065741bf2d 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5287,6 +5287,36 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       // Check for auto functions and trailing return type and adjust the
       // return type accordingly.
       if (!D.isInvalidType()) {
+        // [dcl.fct]p6:
+        //
+        // An explicit-object-parameter-declaration is a parameter-declaration
+        // with a this specifier. An explicit-object-parameter-declaration shall
+        // appear only as the first parameter-declaration of a
+        // parameter-declaration-list of either:
+        //
+        //   - a member-declarator that declares a member function [class.mem], or
+        //   - a lambda-declarator [expr.prim.lambda].
+        DeclaratorContext C = D.getContext();
+        ParmVarDecl *First =
+            FTI.NumParams
+                ? dyn_cast_if_present<ParmVarDecl>(FTI.Params[0].Param)
+                : nullptr;
+        if (First && First->isExplicitObjectParameter() &&
+            C != DeclaratorContext::LambdaExpr &&
+
+            // Either not a member or nested declarator in a member.
+            (C != DeclaratorContext::Member ||
+             D.getInnermostNonParenChunk() != &DeclType) &&
+
+            // Allow out-of-line definitions if we have a scope spec.
+            D.getCXXScopeSpec().isEmpty()) {
+          S.Diag(First->getBeginLoc(),
+                 diag::err_explicit_object_parameter_invalid)
+              << First->getSourceRange();
+          D.setInvalidType();
+          AreDeclaratorChunksValid = false;
+        }
+
         // trailing-return-type is only required if we're declaring a function,
         // and not, for instance, a pointer to a function.
         if (D.getDeclSpec().hasAutoTypeSpec() &&
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index 5f29a955e053c..ddb770a92db0f 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -5,7 +5,7 @@
 void f(this); // expected-error{{variable has incomplete type 'void'}} \
               // expected-error{{invalid use of 'this' outside of a non-static member function}}
 
-void g(this auto); // expected-error{{an explicit object parameter cannot appear in a non-member function}}
+void g(this auto); // expected-error{{an explicit object parameter is not allowed here}}
 
 auto l1 = [] (this auto) static {}; // expected-error{{an explicit object parameter cannot appear in a static lambda}}
 auto l2 = [] (this auto) mutable {}; // expected-error{{a lambda with an explicit object parameter cannot be mutable}}
@@ -685,7 +685,7 @@ struct S {
   static void j(this S s); // expected-error {{an explicit object parameter cannot appear in a static function}}
 };
 
-void nonmember(this S s); // expected-error {{an explicit object parameter cannot appear in a non-member function}}
+void nonmember(this S s); // expected-error {{an explicit object parameter is not allowed here}}
 
 int test() {
   S s;
@@ -838,3 +838,21 @@ int h() {
   }();
 }
 }
+
+namespace GH85992 {
+struct S {
+  int (S::*x)(this int); // expected-error {{an explicit object parameter is not allowed here}}
+  int (*y)(this int); // expected-error {{an explicit object parameter is not allowed here}}
+  int (***z)(this int); // expected-error {{an explicit object parameter is not allowed here}}
+
+  int f(this S);
+  int ((g))(this S);
+  int h(int x, int (*)(this S)); // expected-error {{an explicit object parameter is not allowed here}}
+};
+
+using T = int (*)(this int); // expected-error {{an explicit object parameter is not allowed here}}
+using U = int (S::*)(this int); // expected-error {{an explicit object parameter is not allowed here}}
+int h(this int); // expected-error {{an explicit object parameter is not allowed here}}
+
+int S::f(this S) { return 1; }
+}

>From c27a3ff93474bc890de2b66ab7f486bb59e3b6d8 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 17 Apr 2024 16:16:47 +0200
Subject: [PATCH 02/10] [NFC] clang-format

---
 clang/lib/Sema/SemaType.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 406065741bf2d..23f2b9d74931b 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5294,7 +5294,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
         // appear only as the first parameter-declaration of a
         // parameter-declaration-list of either:
         //
-        //   - a member-declarator that declares a member function [class.mem], or
+        //   - a member-declarator that declares a member function [class.mem],
+        //   or
         //   - a lambda-declarator [expr.prim.lambda].
         DeclaratorContext C = D.getContext();
         ParmVarDecl *First =

>From 0c1239a85159aaa4958bb574a326e85f50719b1b Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 17 Apr 2024 16:23:15 +0200
Subject: [PATCH 03/10] [NFC] Reformat comment

---
 clang/lib/Sema/SemaType.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 23f2b9d74931b..95257b801c8a6 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5294,9 +5294,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
         // appear only as the first parameter-declaration of a
         // parameter-declaration-list of either:
         //
-        //   - a member-declarator that declares a member function [class.mem],
-        //   or
-        //   - a lambda-declarator [expr.prim.lambda].
+        // - a member-declarator that declares a member function [class.mem], or
+        // - a lambda-declarator [expr.prim.lambda].
         DeclaratorContext C = D.getContext();
         ParmVarDecl *First =
             FTI.NumParams

>From 8edd57fd41c5c62de628b8ea9cf6b0ff1e4bc535 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Tue, 23 Apr 2024 18:25:04 +0200
Subject: [PATCH 04/10] [Clang] Check that the NNS is a class type

---
 clang/lib/Sema/SemaType.cpp                | 9 ++++++++-
 clang/test/SemaCXX/cxx2b-deducing-this.cpp | 5 +++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 95257b801c8a6..d8b74b640af40 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5287,6 +5287,13 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       // Check for auto functions and trailing return type and adjust the
       // return type accordingly.
       if (!D.isInvalidType()) {
+        auto isClassType = [&](CXXScopeSpec& SS) {
+          // If there already was an problem with the scope; don’t issue another
+          // error about the explicit object parameter.
+          return SS.isInvalid() ||
+                 isa_and_present<CXXRecordDecl>(S.computeDeclContext(SS));
+        };
+
         // [dcl.fct]p6:
         //
         // An explicit-object-parameter-declaration is a parameter-declaration
@@ -5309,7 +5316,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
              D.getInnermostNonParenChunk() != &DeclType) &&
 
             // Allow out-of-line definitions if we have a scope spec.
-            D.getCXXScopeSpec().isEmpty()) {
+            !isClassType(D.getCXXScopeSpec())) {
           S.Diag(First->getBeginLoc(),
                  diag::err_explicit_object_parameter_invalid)
               << First->getSourceRange();
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index ddb770a92db0f..d2c47b77365cd 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -855,4 +855,9 @@ using U = int (S::*)(this int); // expected-error {{an explicit object parameter
 int h(this int); // expected-error {{an explicit object parameter is not allowed here}}
 
 int S::f(this S) { return 1; }
+
+namespace a {
+void f();
+};
+void a::f(this auto) {} // expected-error {{an explicit object parameter is not allowed here}}
 }

>From 9c0609cf787cf00685d05ecabf707a885a77cc73 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Tue, 23 Apr 2024 19:09:45 +0200
Subject: [PATCH 05/10] [Clang] Handle friends and restore better diagnostic

---
 clang/lib/Sema/SemaDeclCXX.cpp             | 17 +++++++++++++++++
 clang/lib/Sema/SemaType.cpp                | 22 +++++++++++++++++-----
 clang/test/SemaCXX/cxx2b-deducing-this.cpp | 15 +++++++++++----
 3 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ea634df48e5de..1333f8492b40d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -11459,6 +11459,23 @@ void Sema::CheckExplicitObjectMemberFunction(Declarator &D,
     D.setInvalidType();
   }
 
+  // Handle the following case:
+  //
+  // struct S {
+  //   struct T {
+  //     int f(this T);
+  //   };
+  //
+  //   friend int T::f(this T); // Allow this.
+  //   friend int f(this S);    // But disallow this.
+  // };
+  if (D.getDeclSpec().isFriendSpecified() && D.getCXXScopeSpec().isEmpty()) {
+    Diag(ExplicitObjectParam->getBeginLoc(),
+         diag::err_explicit_object_parameter_nonmember)
+        << D.getSourceRange() << /*non-member=*/2 << IsLambda;
+    D.setInvalidType();
+  }
+
   if (IsLambda && FTI.hasMutableQualifier()) {
     Diag(ExplicitObjectParam->getBeginLoc(),
          diag::err_explicit_object_parameter_mutable)
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index d8b74b640af40..2c2bdb503a34c 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5308,18 +5308,30 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
             FTI.NumParams
                 ? dyn_cast_if_present<ParmVarDecl>(FTI.Params[0].Param)
                 : nullptr;
+
+        auto IsFunctionDecl = D.getInnermostNonParenChunk() == &DeclType;
         if (First && First->isExplicitObjectParameter() &&
             C != DeclaratorContext::LambdaExpr &&
 
             // Either not a member or nested declarator in a member.
-            (C != DeclaratorContext::Member ||
-             D.getInnermostNonParenChunk() != &DeclType) &&
+            //
+            // Note that e.g. 'static' or 'friend' declarations are accepted
+            // here; we diagnose them later when we build the member function
+            // because it's easier that way.
+            (C != DeclaratorContext::Member || !IsFunctionDecl) &&
 
             // Allow out-of-line definitions if we have a scope spec.
             !isClassType(D.getCXXScopeSpec())) {
-          S.Diag(First->getBeginLoc(),
-                 diag::err_explicit_object_parameter_invalid)
-              << First->getSourceRange();
+          if (IsFunctionDecl)
+            S.Diag(First->getBeginLoc(),
+                   diag::err_explicit_object_parameter_nonmember)
+                << /*non-member*/ 2 << /*function*/ 0
+                << First->getSourceRange();
+          else
+            S.Diag(First->getBeginLoc(),
+                   diag::err_explicit_object_parameter_invalid)
+                << First->getSourceRange();
+
           D.setInvalidType();
           AreDeclaratorChunksValid = false;
         }
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index d2c47b77365cd..48f963acc27e6 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -5,7 +5,7 @@
 void f(this); // expected-error{{variable has incomplete type 'void'}} \
               // expected-error{{invalid use of 'this' outside of a non-static member function}}
 
-void g(this auto); // expected-error{{an explicit object parameter is not allowed here}}
+void g(this auto); // expected-error{{an explicit object parameter cannot appear in a non-member function}}
 
 auto l1 = [] (this auto) static {}; // expected-error{{an explicit object parameter cannot appear in a static lambda}}
 auto l2 = [] (this auto) mutable {}; // expected-error{{a lambda with an explicit object parameter cannot be mutable}}
@@ -685,7 +685,7 @@ struct S {
   static void j(this S s); // expected-error {{an explicit object parameter cannot appear in a static function}}
 };
 
-void nonmember(this S s); // expected-error {{an explicit object parameter is not allowed here}}
+void nonmember(this S s); // expected-error{{an explicit object parameter cannot appear in a non-member function}}
 
 int test() {
   S s;
@@ -847,17 +847,24 @@ struct S {
 
   int f(this S);
   int ((g))(this S);
+  friend int h(this S); // expected-error {{an explicit object parameter cannot appear in a non-member function}}
   int h(int x, int (*)(this S)); // expected-error {{an explicit object parameter is not allowed here}}
+
+  struct T {
+    int f(this T);
+  };
+
+  friend int T::f(this T);
 };
 
 using T = int (*)(this int); // expected-error {{an explicit object parameter is not allowed here}}
 using U = int (S::*)(this int); // expected-error {{an explicit object parameter is not allowed here}}
-int h(this int); // expected-error {{an explicit object parameter is not allowed here}}
+int h(this int); // expected-error {{an explicit object parameter cannot appear in a non-member function}}
 
 int S::f(this S) { return 1; }
 
 namespace a {
 void f();
 };
-void a::f(this auto) {} // expected-error {{an explicit object parameter is not allowed here}}
+void a::f(this auto) {} // expected-error {{an explicit object parameter cannot appear in a non-member function}}
 }

>From 853da04fe387f56fa69a468342bb665381db2b79 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Tue, 23 Apr 2024 19:10:11 +0200
Subject: [PATCH 06/10] [NFC] clang-format

---
 clang/lib/Sema/SemaType.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 2c2bdb503a34c..6a7754dce96d7 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5287,7 +5287,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       // Check for auto functions and trailing return type and adjust the
       // return type accordingly.
       if (!D.isInvalidType()) {
-        auto isClassType = [&](CXXScopeSpec& SS) {
+        auto isClassType = [&](CXXScopeSpec &SS) {
           // If there already was an problem with the scope; don’t issue another
           // error about the explicit object parameter.
           return SS.isInvalid() ||

>From 3d91f2fdade769e80251f831926dd8529ff0d99c Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Tue, 23 Apr 2024 19:11:28 +0200
Subject: [PATCH 07/10] [NFC] Remove spurious whitespace change

---
 clang/test/SemaCXX/cxx2b-deducing-this.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index 48f963acc27e6..7339ffeb2dab3 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -685,7 +685,7 @@ struct S {
   static void j(this S s); // expected-error {{an explicit object parameter cannot appear in a static function}}
 };
 
-void nonmember(this S s); // expected-error{{an explicit object parameter cannot appear in a non-member function}}
+void nonmember(this S s); // expected-error {{an explicit object parameter cannot appear in a non-member function}}
 
 int test() {
   S s;

>From e3ea2c650c749d4401613b780c912a12dd998f20 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Tue, 23 Apr 2024 19:19:08 +0200
Subject: [PATCH 08/10] [NFC] Update comments

---
 clang/lib/Sema/SemaType.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 6a7754dce96d7..ac6c93c04f925 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5288,7 +5288,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       // return type accordingly.
       if (!D.isInvalidType()) {
         auto isClassType = [&](CXXScopeSpec &SS) {
-          // If there already was an problem with the scope; don’t issue another
+          // If there already was an problem with the scope, don’t issue another
           // error about the explicit object parameter.
           return SS.isInvalid() ||
                  isa_and_present<CXXRecordDecl>(S.computeDeclContext(SS));
@@ -5320,7 +5320,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
             // because it's easier that way.
             (C != DeclaratorContext::Member || !IsFunctionDecl) &&
 
-            // Allow out-of-line definitions if we have a scope spec.
+            // Allow out-of-line definitions of member functions.
             !isClassType(D.getCXXScopeSpec())) {
           if (IsFunctionDecl)
             S.Diag(First->getBeginLoc(),

>From 985044944bd21e1a08a8c204a01974c78df18e37 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Tue, 23 Apr 2024 19:23:28 +0200
Subject: [PATCH 09/10] [Docs] Add release note

---
 clang/docs/ReleaseNotes.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ade8f4e93d5a0..4d926d7737670 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -539,6 +539,8 @@ Bug Fixes to C++ Support
 - Clang now correctly tracks type dependence of by-value captures in lambdas with an explicit
   object parameter.
   Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), (#GH86398), and (#GH86399).
+- Clang now diagnoses explicit object parameters in member pointers and other contexts where they should not appear.
+  Fixes (#GH85992).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

>From 1d307fef1d9d6e1bee52c5d813f5c5f7805c87d6 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Tue, 28 May 2024 20:08:57 +0200
Subject: [PATCH 10/10] Update clang/lib/Sema/SemaType.cpp

Co-authored-by: Aaron Ballman <aaron at aaronballman.com>
---
 clang/lib/Sema/SemaType.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index f371301e89bec..27e39ec4e195d 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -4867,7 +4867,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
                 ? dyn_cast_if_present<ParmVarDecl>(FTI.Params[0].Param)
                 : nullptr;
 
-        auto IsFunctionDecl = D.getInnermostNonParenChunk() == &DeclType;
+        bool IsFunctionDecl = D.getInnermostNonParenChunk() == &DeclType;
         if (First && First->isExplicitObjectParameter() &&
             C != DeclaratorContext::LambdaExpr &&
 



More information about the cfe-commits mailing list