[Clang] Disallow explicit object parameters in more contexts #89078

Sirraide · 2024-04-17T14:26:38Z

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

The intent of this pr is to disallow explicit object parameters in all other contexts early on when we handle the function declarator.

This fixes #85992.

llvmbot · 2024-04-17T14:27:11Z

@llvm/pr-subscribers-clang

Author: None (Sirraide)

Changes

[[dcl.fct]p6](https://eel.is/c++draft/dcl.fct#6) states:

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

The intent of this pr is to disallow explicit object parameters in all other contexts early on when we handle the function declarator.

This fixes #85992.

Full diff: https://github.com/llvm/llvm-project/pull/89078.diff

4 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
(modified) clang/lib/Sema/SemaDeclCXX.cpp (+2-4)
(modified) clang/lib/Sema/SemaType.cpp (+30)
(modified) clang/test/SemaCXX/cxx2b-deducing-this.cpp (+20-2)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5ec0218aedfe86..680638d2e59c6a 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 7669171fea56ff..ea634df48e5deb 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 404c4e8e31b558..95257b801c8a6c 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 5f29a955e053c3..ddb770a92db0f9 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; }
+}

cor3ntin

Generally, I like the approach.

Note this PR is related to
https://github.com/llvm/llvm-project/pull/88974/files

@sdkrystian

clang/test/SemaCXX/cxx2b-deducing-this.cpp

clang/lib/Sema/SemaType.cpp

Sirraide · 2024-04-17T14:51:21Z

Note this PR is related to
https://github.com/llvm/llvm-project/pull/88974/files

That’s pr covers a different code path from what I can tell though (unless requires expressions are also modelled as function declarators, I’m honestly not sure about that one).

It also reminds me that this still needs a release note.

sdkrystian · 2024-04-17T14:51:40Z

I'm going to merge #88974 shortly, but I don't think this interferes with anything here

Sirraide · 2024-04-17T14:53:32Z

I'm going to merge #88974 shortly, but I don't think this interferes with anything here

Yeah, it seems orthogonal to this patch from what I can tell at least.

Sirraide · 2024-04-23T17:14:28Z

Alright, it took me a bit, but I’ve figured out how to continue issuing the ‘... cannot appear in a non-member function’ diagnostic where appropriate. This also handles friend declarations now because I had forgotten about that; I’ve also added a few more tests for declarations w/ nested-name-specifiers.

Sirraide · 2024-05-20T16:02:45Z

CI failure before seems to have been due to us running out of memory compiling flang from what I can tell?

Sirraide · 2024-05-27T14:00:15Z

ping

clang/include/clang/Basic/DiagnosticSemaKinds.td

clang/lib/Sema/SemaType.cpp

Co-authored-by: Aaron Ballman <[email protected]>

clang/lib/Sema/SemaDeclCXX.cpp

Sirraide · 2024-06-27T22:20:07Z

ping

Sirraide · 2024-07-04T04:47:55Z

ping

AaronBallman

LGTM -- @cor3ntin, are you happy with the changes as well?

cor3ntin

LGTM modulo nit
Thanks for working on this

clang/lib/Sema/SemaType.cpp

Co-authored-by: cor3ntin <[email protected]>

Summary: This diagnoses explicit object parameters in more contexts where they aren’t supposed to appear in (e.g. function pointer types, non-function member decls, etc.) [dcl.fct] This fixes #85992. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251719

Sirraide added 3 commits April 17, 2024 16:15

[Clang] Disallow explicit object parameters in more contexts

c611122

[NFC] clang-format

c27a3ff

[NFC] Reformat comment

0c1239a

Sirraide added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Apr 17, 2024

Sirraide requested review from cor3ntin and AaronBallman April 17, 2024 14:26

llvmbot added the clang Clang issues not falling into any other category label Apr 17, 2024

cor3ntin reviewed Apr 17, 2024

View reviewed changes

clang/test/SemaCXX/cxx2b-deducing-this.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaType.cpp Outdated Show resolved Hide resolved

Sirraide added 4 commits April 23, 2024 18:25

[Clang] Check that the NNS is a class type

8edd57f

[Clang] Handle friends and restore better diagnostic

9c0609c

[NFC] clang-format

853da04

[NFC] Remove spurious whitespace change

3d91f2f

Sirraide requested a review from cor3ntin April 23, 2024 17:14

Sirraide added 3 commits April 23, 2024 19:19

[NFC] Update comments

e3ea2c6

[Docs] Add release note

9850449

Merge branch 'main' into deducing-this-nonmember

42be623

AaronBallman reviewed May 28, 2024

View reviewed changes

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved

clang/lib/Sema/SemaType.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaType.cpp Outdated Show resolved Hide resolved

Update clang/lib/Sema/SemaType.cpp

1d307fe

Co-authored-by: Aaron Ballman <[email protected]>

cor3ntin reviewed May 28, 2024

View reviewed changes

clang/lib/Sema/SemaDeclCXX.cpp Outdated Show resolved Hide resolved

Sirraide added 3 commits June 19, 2024 14:53

Merge branch 'main' into deducing-this-nonmember

40206d3

Handle namespace scope specifiers correctly

aa4f66b

Improve diagnostic wording a bit

a7cf358

AaronBallman approved these changes Jul 12, 2024

View reviewed changes

cor3ntin approved these changes Jul 12, 2024

View reviewed changes

clang/lib/Sema/SemaType.cpp Outdated Show resolved Hide resolved

Sirraide and others added 2 commits July 15, 2024 17:57

Update clang/lib/Sema/SemaType.cpp

f101a97

Co-authored-by: cor3ntin <[email protected]>

Merge branch 'main' into deducing-this-nonmember

2877851

Sirraide merged commit 864478c into llvm:main Jul 15, 2024
8 checks passed

Sirraide deleted the deducing-this-nonmember branch July 16, 2024 00:37

[Clang] Disallow explicit object parameters in more contexts #89078

[Clang] Disallow explicit object parameters in more contexts #89078

Uh oh!

Conversation

Sirraide commented Apr 17, 2024

Uh oh!

llvmbot commented Apr 17, 2024

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Sirraide commented Apr 17, 2024

Uh oh!

sdkrystian commented Apr 17, 2024

Uh oh!

Sirraide commented Apr 17, 2024

Uh oh!

Sirraide commented Apr 23, 2024

Uh oh!

Sirraide commented May 20, 2024

Uh oh!

Sirraide commented May 27, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Sirraide commented Jun 27, 2024

Uh oh!

Sirraide commented Jul 4, 2024

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!