[PATCH] c++: reject packs on xobj params. [PR113307]
Bootstrapped and tested on x86_64-linux with no regressions.
I'm still getting used to things so let me know if the change log
entries are excessive, thanks.From 9dc168e7bcbbd7d515fa28cb9cae28ec113fae0f Mon Sep 17 00:00:00 2001
From: Waffl3x
Date: Thu, 11 Jan 2024 14:32:46 -0700
Subject: [PATCH] c++: reject packs on xobj params. [PR113307]
Reject and diagnose xobj parameters declared as parameter packs.
PR c++/113307
gcc/cp/ChangeLog:
* parser.cc (cp_parser_parameter_declaration): Reject packs
on xobj params.
gcc/testsuite/ChangeLog:
* g++.dg/cpp23/explicit-obj-diagnostics3.C: Add test for
rejection of packs.
Signed-off-by: Waffl3x
---
gcc/cp/parser.cc | 21 +++-
.../g++.dg/cpp23/explicit-obj-diagnostics3.C | 106 +-
2 files changed, 125 insertions(+), 2 deletions(-)
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 8ab98cc0c23..70fbba09bf8 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -25706,6 +25706,25 @@ cp_parser_parameter_declaration (cp_parser *parser,
for a C-style variadic function. */
token = cp_lexer_peek_token (parser->lexer);
+ bool const xobj_param_p
+= decl_spec_seq_has_spec_p (_specifiers, ds_this);
+
+ if (xobj_param_p
+ && ((declarator && declarator->parameter_pack_p)
+ || cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS)))
+{
+ location_t xobj_param
+ = make_location (decl_specifiers.locations[ds_this],
+ decl_spec_token_start->location,
+ input_location);
+ error_at(xobj_param,
+ "an explicit object parameter cannot "
+ "be a function parameter pack");
+ /* Suppress errors that occur down the line. */
+ if (declarator)
+ declarator->parameter_pack_p = false;
+}
+
/* If a function parameter pack was specified and an implicit template
parameter was introduced during cp_parser_parameter_declaration,
change any implicit parameters introduced into packs. */
@@ -25829,7 +25848,7 @@ cp_parser_parameter_declaration (cp_parser *parser,
if (default_argument)
STRIP_ANY_LOCATION_WRAPPER (default_argument);
- if (decl_spec_seq_has_spec_p (_specifiers, ds_this))
+ if (xobj_param_p)
{
if (default_argument)
{
diff --git a/gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics3.C b/gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics3.C
index ec091d6ca67..304cf029f8f 100644
--- a/gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics3.C
+++ b/gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics3.C
@@ -1,7 +1,9 @@
// P0847R7
// { dg-do compile { target c++23 } }
-// rejection and diagnosis of an xobj parameter declared with a default argument
+// rejection and diagnosis of an incorrectly declared xobj parameter
+
+// default argument
struct S {
void f0(this S = {}) {} // { dg-error "an explicit object parameter may not have a default argument" }
@@ -18,3 +20,105 @@ void S::f2(this S = {}) {} // { dg-error "an explicit object parameter may not h
void S::f11(this S s) {}
void S::f12(this S s = {}) {} // { dg-error "an explicit object parameter may not have a default argument" }
+// parameter pack
+
+struct S0 {
+ template
+ void f(this Selves...) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ template
+ void g(this Selves... selves) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ void h(this auto...) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+ void j(this auto... selves) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ template
+ void fd(this Selves...); // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ template
+ void gd(this Selves... selves); // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ void hd(this auto...); // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+ void jd(this auto... selves); // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+};
+
+struct S1 {
+ template
+ void f(this Selves&...) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ template
+ void g(this Selves&... selves) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ void h(this auto&...) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+ void j(this auto&... selves) {} // { dg-error "an explicit object parameter cannot be a function parameter pack" }
+
+ template
+ void fd(this Selves&...); // { dg-error "an explicit object parameter cannot be a function parameter p
Re: [PATCH v8 1/4] c++: P0847R7 (deducing this) - prerequisite changes. [PR102609]
On Tuesday, January 9th, 2024 at 3:52 PM, Jason Merrill
wrote:
>
>
> On 1/9/24 17:34, waffl3x wrote:
>
> > On Tuesday, January 9th, 2024 at 2:56 PM, Jason Merrill ja...@redhat.com
> > wrote:
> >
> >
> > Is the type of an implicit object parameter specified elsewhere? I have
> > looked for it more than once and I could only find that passage, but I
> > guess [over.match.funcs-1] is what I missed here, so
> > [over.match.funcs-4] doesn't apply elsewhere.
>
>
> That's the only definition I know of. And indeed my statement was
> wrong, it also affects the considerations in add_method. But I still
> think it would be more complicated in more places to deal with proxy
> FUNCTION_DECLs (like we do for inheriting constructors).
You would know the code base better than I, so you're probably right.
The code in add_method probably already handles it just fine, the only
caveat being that it needs corresponding object parameters when
deciding to discard an overload introduced by a using declaration...
uhoh yeah that might not be okay. This probably results in ambiguous
overload resolution, I'll throw together a test case to see if I can
confirm this...
struct B {
void f() {}
};
struct S : B {
using B::f;
void f(this S&) {}
};
int main() {
S s{};
s.f();
}
And yep, you're right, add_method is going to need to care about this
too. In the above test case, the call to f is incorrectly ambiguous.
The overload of f introduced by the using declaration should have been
discarded as the object parameters should correspond.
struct S;
struct B {
void g(this S&) {}
};
struct S : B {
using B::g;
void g() {}
};
int main()
{
S s{};
s.g();
}
While this case works correctly, the overload of g introduced by the
using declaration is correctly discarded.
Hopefully it's not too annoying to solve, I'm not exactly sure how to
go about it (other than the method you've already turned down) so I'll
leave it to you.
Alex
Re: [PATCH v8 1/4] c++: P0847R7 (deducing this) - prerequisite changes. [PR102609]
On Tuesday, January 9th, 2024 at 2:56 PM, Jason Merrill wrote: > > > On 1/9/24 15:58, Jason Merrill wrote: > > > On 1/6/24 19:00, waffl3x wrote: > > > > > Bootstrapped and tested on x86_64-linux with no regressions. > > > > > > I'm considering this finished, I have CWG2586 working but I have not > > > included it in this version of the patch. I was not happy with the > > > amount of work I had done on it. I will try to get it finished before > > > we get cut off, and I'm pretty sure I can. I just don't want to risk > > > missing the boat for the whole patch just for that. > > > > > > There aren't too many changes from v7, it's mostly just cleaned up. > > > There are a few though, so do take a look, if there's anything severe I > > > can rush to fix it if necessary. > > > > > > That's all, hopefully all is good, fingers crossed. > > > > Great. Given where we are in the release cycle, I'm thinking to put > > these with only minimal changes and then do any further adjustments > > afterward. > > > > For this first one, I needed to fix the commit message to wrap at 75 > > columns so that it fits in 80 columns with the initial padding added by > > 'git log'. I also needed to adjust the ChangeLog entry to please git > > gcc-verify. And I changed the credit note to a Co-authored-by. > > > > The other commit messages only needed wrapping. > > > I just pushed your patches along with these two follow-ons: Is the type of an implicit object parameter specified elsewhere? I have looked for it more than once and I could only find that passage, but I guess [over.match.funcs-1] is what I missed here, so [over.match.funcs-4] doesn't apply elsewhere. I have no objection to changing the comment in iobj_xobj_parameters_correspond if you have a better idea on how to fix the bug that inspired that comment. Especially since the standard is on your side for it. In this case, perhaps the comment should instead say something along the lines of "this function does not account for using declarations." That does get a little hairy though if both an iobj and xobj member function are introduced by a using declaration. Perhaps we should just optionally take a type that we can compare against? That sounds really icky, maybe the solution you have in mind will work just fine. BTW, I definitely assumed TYPE_MAIN_VARIANT was what I wanted in other places in the patch, so there might be latent errors waiting to happen. It's probably fine because those areas where I used them are pretty well tested, but I thought it would be worth mentioning just in case. I'm very excited to see this get through, thanks again for all the help and patience. Alex
Re: [PATCH v8 1/4] c++: P0847R7 (deducing this) - prerequisite changes. [PR102609]
On Tuesday, January 9th, 2024 at 1:58 PM, Jason Merrill wrote: > > > On 1/6/24 19:00, waffl3x wrote: > > > Bootstrapped and tested on x86_64-linux with no regressions. > > > > I'm considering this finished, I have CWG2586 working but I have not > > included it in this version of the patch. I was not happy with the > > amount of work I had done on it. I will try to get it finished before > > we get cut off, and I'm pretty sure I can. I just don't want to risk > > missing the boat for the whole patch just for that. > > > > There aren't too many changes from v7, it's mostly just cleaned up. > > There are a few though, so do take a look, if there's anything severe I > > can rush to fix it if necessary. > > > > That's all, hopefully all is good, fingers crossed. > > > Great. Given where we are in the release cycle, I'm thinking to put > these with only minimal changes and then do any further adjustments > afterward. > > For this first one, I needed to fix the commit message to wrap at 75 > columns so that it fits in 80 columns with the initial padding added by > 'git log'. I also needed to adjust the ChangeLog entry to please git > gcc-verify. And I changed the credit note to a Co-authored-by. > > The other commit messages only needed wrapping. > > Thanks! > > Jason Sounds good to me, it's been a pleasure working with you. If there's anything you would like me to do differently for future patches just let me know. For now, I think I will take a break. Alex
[PATCH v8 5/4] c++: P0847R7 (deducing this) - CWG2586. [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions.
Not as hard as I thought it would be! As noted in the commit message, I believe
this makes explicit object member functions feature complete.From a5f947d411b5e19ce7efbb4d766a2792b02c9626 Mon Sep 17 00:00:00 2001
From: Waffl3x
Date: Sun, 7 Jan 2024 15:02:57 -0700
Subject: [PATCH] C++23 P0847R7 (deducing this) - CWG2586. [PR102609]
This adds support for defaulted comparison operators and copy/move assignment
operators, as well as allowing user defined xobj copy/move assignment
operators. It turns out defaulted comparison operators already worked though,
so this just adds a test for them. Defaulted comparison operators were not so
nice and required a bit of a hack. Should work fine though!
The diagnostics leave something to be desired, and there are some things that
could be improved with more extensive design changes. There are a few notes
left indicating where I think we could make improvements.
Aside from some small bugs, with this commit xobj member functions should be
feature complete.
PR c++/102609
gcc/cp/ChangeLog:
PR c++/102609
C++23 P0847R7 (deducing this) - CWG2586.
* decl.cc (copy_fn_p): Accept xobj copy assignment functions.
(move_signature_fn_p): Accept xobj move assignment functions.
* method.cc (do_build_copy_assign): Handle defaulted xobj member
functions.
(defaulted_late_check): Comment.
(defaultable_fn_check): Comment.
gcc/testsuite/ChangeLog:
PR c++/102609
C++23 P0847R7 (deducing this) - CWG2586.
* g++.dg/cpp23/explicit-obj-basic6.C: New test.
* g++.dg/cpp23/explicit-obj-default1.C: New test.
* g++.dg/cpp23/explicit-obj-default2.C: New test.
Signed-off-by: Waffl3x
---
gcc/cp/decl.cc| 28 +++-
gcc/cp/method.cc | 55 ++--
.../g++.dg/cpp23/explicit-obj-basic6.C| 51 +++
.../g++.dg/cpp23/explicit-obj-default1.C | 57
.../g++.dg/cpp23/explicit-obj-default2.C | 65 +++
5 files changed, 248 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-basic6.C
create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-default1.C
create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-default2.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 7f267055c29..b10a72a87bf 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -15663,7 +15663,19 @@ copy_fn_p (const_tree d)
&& DECL_NAME (d) != assign_op_identifier)
return 0;
- args = FUNCTION_FIRST_USER_PARMTYPE (d);
+ if (DECL_XOBJ_MEMBER_FUNCTION_P (d))
+{
+ tree object_param = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (d)));
+ if (!TYPE_REF_P (object_param)
+ || TYPE_REF_IS_RVALUE (object_param)
+ /* Reject unrelated object parameters. */
+ || TYPE_MAIN_VARIANT (TREE_TYPE (object_param)) != DECL_CONTEXT (d)
+ || CP_TYPE_CONST_P (TREE_TYPE (object_param)))
+ return 0;
+ args = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (d)));
+}
+ else
+args = FUNCTION_FIRST_USER_PARMTYPE (d);
if (!args)
return 0;
@@ -15738,7 +15750,19 @@ move_signature_fn_p (const_tree d)
&& DECL_NAME (d) != assign_op_identifier)
return 0;
- args = FUNCTION_FIRST_USER_PARMTYPE (d);
+ if (DECL_XOBJ_MEMBER_FUNCTION_P (d))
+{
+ tree object_param = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (d)));
+ if (!TYPE_REF_P (object_param)
+ || TYPE_REF_IS_RVALUE (object_param)
+ /* Reject unrelated object parameters. */
+ || TYPE_MAIN_VARIANT (TREE_TYPE (object_param)) != DECL_CONTEXT (d)
+ || CP_TYPE_CONST_P (TREE_TYPE (object_param)))
+ return 0;
+ args = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (d)));
+}
+ else
+args = FUNCTION_FIRST_USER_PARMTYPE (d);
if (!args)
return 0;
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index aa5a044883e..da6a08a0304 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -795,13 +795,19 @@ do_build_copy_assign (tree fndecl)
compound_stmt = begin_compound_stmt (0);
parm = convert_from_reference (parm);
+ /* If we are building a defaulted xobj copy/move assignment operator then
+ current_class_ref will not have been set up.
+ Kind of an icky hack, but what can ya do? */
+ tree const class_ref = DECL_XOBJ_MEMBER_FUNCTION_P (fndecl)
+? cp_build_fold_indirect_ref (DECL_ARGUMENTS (fndecl)) : current_class_ref;
+
if (trivial
&& is_empty_class (current_class_type))
/* Don't copy the padding byte; it might not have been allocated
if *this is a base subobject. */;
else if (trivial)
{
- tree t = build2 (MODIFY_EXPR, void_type_node, current_class_ref, parm);
+ tree t = build2 (MODIFY_EXPR, void_type_node, class_ref, parm);
finish_expr_stmt (t);
}
else
@@ -826,7 +832,7 @@ do_build_copy_assign (tree fndecl)
/* Call the base class assignment operator. */
releasing_vec parmvec (make_tree_vecto
Re: [PATCH v8 4/4] c++: P0847R7 (deducing this) - xobj lambdas. [PR102609]
On Saturday, January 6th, 2024 at 5:17 PM, Jakub Jelinek wrote: > > > On Sun, Jan 07, 2024 at 12:05:50AM +, waffl3x wrote: > > > Bootstrapped and tested on x86_64-linux with no regressions. > > > > That's it for now. If I manage to finish CWG2586 in time I guess I'll > > submit it as patch 5/4? I'm definitely locked in on all these changes > > unless there's a really good reason. > > > No patch attached nor included here... > > Jakub MY BAD, thank you very much. Alex From aac380e6f7e7fddfa7acf3c166a75bda56d54f7a Mon Sep 17 00:00:00 2001 From: Waffl3x Date: Sat, 6 Jan 2024 16:29:45 -0700 Subject: [PATCH 4/4] C++23 P0847R7 (deducing this) - xobj lambdas. [PR102609] This implements support for xobj lambdas. There are extensive tests included, but not exhaustive. Dependent lambdas should work and have been tested lightly, but we need more exhaustive tests for them. PR c++/102609 gcc/cp/ChangeLog: PR c++/102609 C++23 P0847R7 (deducing this) - xobj lambdas. * lambda.cc (build_capture_proxy): Don't fold direct object types. * parser.cc (cp_parser_lambda_declarator_opt): Handle xobj lambdas, diagnostics. Comments also updated. * pt.cc (tsubst_function_decl): Handle xobj lambdas. Check object type of xobj lambda call operator, diagnose incorrect types. (tsubst_lambda_expr): Update comment. * semantics.cc (finish_decltype_type): Also consider by-value object parameter qualifications. gcc/testsuite/ChangeLog: PR c++/102609 C++23 P0847R7 (deducing this) - xobj lambdas. * g++.dg/cpp23/explicit-obj-diagnostics8.C: New test. * g++.dg/cpp23/explicit-obj-lambda1.C: New test. * g++.dg/cpp23/explicit-obj-lambda10.C: New test. * g++.dg/cpp23/explicit-obj-lambda11.C: New test. * g++.dg/cpp23/explicit-obj-lambda12.C: New test. * g++.dg/cpp23/explicit-obj-lambda13.C: New test. * g++.dg/cpp23/explicit-obj-lambda2.C: New test. * g++.dg/cpp23/explicit-obj-lambda3.C: New test. * g++.dg/cpp23/explicit-obj-lambda4.C: New test. * g++.dg/cpp23/explicit-obj-lambda5.C: New test. * g++.dg/cpp23/explicit-obj-lambda6.C: New test. * g++.dg/cpp23/explicit-obj-lambda7.C: New test. * g++.dg/cpp23/explicit-obj-lambda8.C: New test. * g++.dg/cpp23/explicit-obj-lambda9.C: New test. Signed-off-by: Waffl3x --- gcc/cp/lambda.cc | 4 +- gcc/cp/parser.cc | 84 +- gcc/cp/pt.cc | 74 +- gcc/cp/semantics.cc | 10 +- .../g++.dg/cpp23/explicit-obj-diagnostics8.C | 68 ++ .../g++.dg/cpp23/explicit-obj-lambda1.C | 25 + .../g++.dg/cpp23/explicit-obj-lambda10.C | 39 + .../g++.dg/cpp23/explicit-obj-lambda11.C | 46 + .../g++.dg/cpp23/explicit-obj-lambda12.C | 103 +++ .../g++.dg/cpp23/explicit-obj-lambda13.C | 103 +++ .../g++.dg/cpp23/explicit-obj-lambda2.C | 23 + .../g++.dg/cpp23/explicit-obj-lambda3.C | 64 ++ .../g++.dg/cpp23/explicit-obj-lambda4.C | 23 + .../g++.dg/cpp23/explicit-obj-lambda5.C | 21 + .../g++.dg/cpp23/explicit-obj-lambda6.C | 873 ++ .../g++.dg/cpp23/explicit-obj-lambda7.C | 20 + .../g++.dg/cpp23/explicit-obj-lambda8.C | 87 ++ .../g++.dg/cpp23/explicit-obj-lambda9.C | 46 + 18 files changed, 1699 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics8.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda1.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda10.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda11.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda12.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda13.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda2.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda3.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda4.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda5.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda6.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda7.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda8.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-lambda9.C diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc index fc6a0708b66..09a9f148252 100644 --- a/gcc/cp/lambda.cc +++ b/gcc/cp/lambda.cc @@ -404,8 +404,10 @@ build_capture_proxy (tree member, tree init) fn = lambda_function (closure); lam = CLASSTYPE_LAMBDA_EXPR (closure); + object = DECL_ARGUMENTS (fn); /* The proxy variable forwards to the capture field. */ - object = build_fold_indirect_ref (DECL_ARGUMENTS (fn)); + if (INDIRECT_TYPE_P (TREE_TYPE (object))) +object = build_fold_indirect_ref (object); object = finish_non_static_data_member (member, object, NULL_TREE); if (REFERE
[PATCH v8 4/4] c++: P0847R7 (deducing this) - xobj lambdas. [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions. That's it for now. If I manage to finish CWG2586 in time I guess I'll submit it as patch 5/4? I'm definitely locked in on all these changes unless there's a really good reason. Alex
[PATCH v8 3/4] c++: P0847R7 (deducing this) - diagnostics. [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions.From 32a713d9826a042b260e84dcfbfd31c619a122fb Mon Sep 17 00:00:00 2001 From: Waffl3x Date: Fri, 5 Jan 2024 14:34:34 -0700 Subject: [PATCH 3/4] C++23 P0847R7 (deducing this) - diagnostics. [PR102609] Diagnostics for xobj member functions. Also includes some diagnostics for xobj lambdas which are not implemented here. CWG2554 is also implemented here, we explicitly error when an xobj member function overrides a virtual function. PR c++/102609 gcc/c-family/ChangeLog: PR c++/102609 C++23 P0847R7 (deducing this) - diagnostics. * c-cppbuiltin.cc (c_cpp_builtins): Define __cpp_explicit_this_parameter=202110L feature test macro. gcc/cp/ChangeLog: PR c++/102609 C++23 P0847R7 (deducing this) - diagnostics. * class.cc (resolve_address_of_overloaded_function): Diagnostics. * cp-tree.h (TFF_XOBJ_FUNC): Define. * decl.cc (grokfndecl): Diagnostics. (grokdeclarator): Diagnostics. * error.cc (dump_aggr_type): Pass TFF_XOBJ_FUNC. (dump_lambda_function): Formatting for xobj lambda. (dump_function_decl): Pass TFF_XOBJ_FUNC. (dump_parameters): Formatting for xobj member functions. (function_category): Formatting for xobj member functions. * parser.cc (cp_parser_decl_specifier_seq): Diagnostics. (cp_parser_parameter_declaration): Diagnostics. * search.cc (look_for_overrides_here): Make xobj member functions override. (look_for_overrides_r): Reject an overriding xobj member function and diagnose it. * semantics.cc (finish_this_expr): Diagnostics. * typeck.cc (cp_build_addr_expr_1): Diagnostics. gcc/testsuite/ChangeLog: PR c++/102609 C++23 P0847R7 (deducing this) - diagnostics. * g++.dg/cpp23/feat-cxx2b.C: Test existance and value of __cpp_explicit_this_parameter feature test macro. * g++.dg/cpp26/feat-cxx26.C: Likewise. * g++.dg/cpp23/explicit-obj-cxx-dialect-A.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-B.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-C.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-D.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-E.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics1.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics2.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics3.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics4.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics5.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics6.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics7.C: New test. Signed-off-by: Waffl3x --- gcc/c-family/c-cppbuiltin.cc | 1 + gcc/cp/class.cc | 55 - gcc/cp/cp-tree.h | 5 +- gcc/cp/decl.cc| 138 ++-- gcc/cp/error.cc | 24 +- gcc/cp/parser.cc | 38 +++- gcc/cp/search.cc | 14 +- gcc/cp/semantics.cc | 25 ++- gcc/cp/typeck.cc | 45 +++- .../g++.dg/cpp23/explicit-obj-cxx-dialect-A.C | 7 + .../g++.dg/cpp23/explicit-obj-cxx-dialect-B.C | 7 + .../g++.dg/cpp23/explicit-obj-cxx-dialect-C.C | 9 + .../g++.dg/cpp23/explicit-obj-cxx-dialect-D.C | 8 + .../g++.dg/cpp23/explicit-obj-cxx-dialect-E.C | 8 + .../g++.dg/cpp23/explicit-obj-diagnostics1.C | 139 .../g++.dg/cpp23/explicit-obj-diagnostics2.C | 26 +++ .../g++.dg/cpp23/explicit-obj-diagnostics3.C | 20 ++ .../g++.dg/cpp23/explicit-obj-diagnostics4.C | 16 ++ .../g++.dg/cpp23/explicit-obj-diagnostics5.C | 23 ++ .../g++.dg/cpp23/explicit-obj-diagnostics6.C | 206 ++ .../g++.dg/cpp23/explicit-obj-diagnostics7.C | 95 gcc/testsuite/g++.dg/cpp23/feat-cxx2b.C | 6 + gcc/testsuite/g++.dg/cpp26/feat-cxx26.C | 6 + 23 files changed, 871 insertions(+), 50 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-cxx-dialect-A.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-cxx-dialect-B.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-cxx-dialect-C.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-cxx-dialect-D.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-cxx-dialect-E.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics1.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics2.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics3.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics4.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics5.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics6.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-obj-diagnostics7.C diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc index 2d1249f29ed..73c14f6d570 100644 --- a/gcc/c-family/c-cppbuiltin.cc +++ b/gcc/c-family/c-cppbuiltin.cc
[PATCH v8 1/4] c++: P0847R7 (deducing this) - prerequisite changes. [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions. I'm considering this finished, I have CWG2586 working but I have not included it in this version of the patch. I was not happy with the amount of work I had done on it. I will try to get it finished before we get cut off, and I'm pretty sure I can. I just don't want to risk missing the boat for the whole patch just for that. There aren't too many changes from v7, it's mostly just cleaned up. There are a few though, so do take a look, if there's anything severe I can rush to fix it if necessary. That's all, hopefully all is good, fingers crossed. AlexFrom cd122053dfad741a7d90adcd45929af768ce643f Mon Sep 17 00:00:00 2001 From: Waffl3x Date: Sun, 31 Dec 2023 03:16:36 -0700 Subject: [PATCH 1/4] C++23 P0847R7 (deducing this) - prerequisite changes. [PR102609] Adds the xobj_flag member to lang_decl_fn and a corresponding member access macro and predicate to support the addition of explicit object member functions. Additionally, since explicit object member functions are also non-static member functions, we need to change uses of DECL_NONSTATIC_MEMBER_FUNCTION_P to clarify whether they intend to include or exclude them. Many of these alterations are authored by Jason Merril. PR c++/102609 gcc/cp/ChangeLog: PR c++/102609 C++23 P0847R7 (deducing this) - prerequisite changes. [PR102609] * cp-tree.h (struct lang_decl_fn): New data member. (DECL_NONSTATIC_MEMBER_FUNCTION_P): Poison. (DECL_IOBJ_MEMBER_FUNCTION_P): Define. (DECL_FUNCTION_XOBJ_FLAG): Define. (DECL_XOBJ_MEMBER_FUNCTION_P): Define. (DECL_OBJECT_MEMBER_FUNCTION_P): Define. (DECL_FUNCTION_MEMBER_P): Don't use DECL_NONSTATIC_MEMBER_FUNCTION_P. (DECL_CONST_MEMFUNC_P): Likewise. (DECL_VOLATILE_MEMFUNC_P): Likewise. (DECL_NONSTATIC_MEMBER_P): Likewise. * module.cc (trees_out::lang_decl_bools): Handle xobj_flag. (trees_in::lang_decl_bools): Handle xobj_flag. * call.cc (build_this_conversion): (add_function_candidate): (add_template_candidate_real): (add_candidates): (maybe_warn_class_memaccess): (cand_parms_match): (joust): (do_warn_dangling_reference): Don't use it. * class.cc (finalize_literal_type_property): (finish_struct): (resolve_address_of_overloaded_function): * constexpr.cc (is_valid_constexpr_fn): (cxx_bind_parameters_in_call): * contracts.cc (build_contract_condition_function): * cp-objcp-common.cc (cp_decl_dwarf_attribute): * cxx-pretty-print.cc (cxx_pretty_printer::postfix_expression): (cxx_pretty_printer::declaration_specifiers): (cxx_pretty_printer::direct_declarator): * decl.cc (cp_finish_decl): (grok_special_member_properties): (start_preparsed_function): (record_key_method_defined): * decl2.cc (cp_handle_deprecated_or_unavailable): * init.cc (find_uninit_fields_r): (build_offset_ref): * lambda.cc (lambda_expr_this_capture): (maybe_generic_this_capture): (nonlambda_method_basetype): * mangle.cc (write_nested_name): * method.cc (early_check_defaulted_comparison): (skip_artificial_parms_for): (num_artificial_parms_for): * module.cc (trees_out::lang_decl_bools): (trees_in::lang_decl_bools): * pt.cc (is_specialization_of_friend): (determine_specialization): (copy_default_args_to_explicit_spec): (check_explicit_specialization): (tsubst_contract_attribute): (check_non_deducible_conversions): (more_specialized_fn): (maybe_instantiate_noexcept): (register_parameter_specializations): (value_dependent_expression_p): * search.cc (shared_member_p): (lookup_member): (field_access_p): * semantics.cc (finish_omp_declare_simd_methods): * tree.cc (lvalue_kind): * typeck.cc (invalid_nonstatic_memfn_p): Don't use DECL_NONSTATIC_MEMBER_FUNCTION_P. libcc1/ChangeLog: PR c++/102609 C++23 P0847R7 (deducing this) - prerequisite changes. [PR102609] * libcp1plugin.cc (plugin_pragma_push_user_expression): Don't use DECL_NONSTATIC_MEMBER_FUNCTION_P. Signed-off-by: Waffl3x --- gcc/cp/call.cc | 25 + gcc/cp/class.cc| 6 +++--- gcc/cp/constexpr.cc| 4 ++-- gcc/cp/contracts.cc| 6 +++--- gcc/cp/cp-objcp-common.cc | 4 ++-- gcc/cp/cp-tree.h | 37 ++--- gcc/cp/cxx-pretty-print.cc | 6 +++--- gcc/cp/decl.cc | 8 gcc/cp/decl2.cc| 2 +- gcc/cp/init.cc | 4 ++-- gcc/cp/lambda.cc | 9 + gcc/cp/mangle.cc | 4 ++-- gcc/cp/method.cc | 8 gcc/cp/module.cc | 6 -- gcc/cp/pt.cc | 38 +++--- gcc/cp/search.cc | 5 +++-- gcc/cp/semantics.cc| 2 +- gcc/cp/tree.cc | 2 +- gcc/cp/typeck.cc | 2 +- libcc1/libcp1plugin.cc | 2 +- 20 files changed, 104 insertions(+), 76 deletions(-) diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 6ac87a298b2..46f2ccdfc48 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -2309,7 +2309,7
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
I found a bugged test while finishing off the patch.
gcc/testsuite/g++.dg/cpp0x/lambda/lambda-type.C
line 72 has
same_type(); // { dg-error "" "not captured" }
The behavior has changed now, and this test should have failed, but
didn't as it has an empty regex expression. The actual error being
emitted here is this:
error: invalid use of incomplete type 'struct same_type'
This still happens to pass because the types don't match, but it should
be fixed to reflect the new intended behavior.
The finished patch will be coming soon(ish), I had to make some changes
so you will have to look over it quickly but hopefully it will be split
up enough to make that convenient.
Alex
On Monday, January 1st, 2024 at 10:12 AM, waffl3x
wrote:
>
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113191
>
> I've posted the report here, I ended up doing a bit more investigation,
> so the contents might interest you. I'm really starting to think that
> we want to do a more thorough re-engineering of how using declarations
> are handled, instead of handling it with little hacks like the one
> added to more_specialized_fn.
>
> As I note in the report, the addition of xobj member functions really
> makes [over.match.funcs.general.4] a lot more relevant, and I don't
> think we can get away with not following it more closely anymore. I
> know I'm wording myself here as if that passage has existed forever,
> but I recognize it might be as recent as C++23 that it was added. I
> don't mean to imply anything with how I'm wording it, it's just way
> easier to express it this way. Especially since we really could get
> away with these kinds of hacks if xobj member functions did not exist.
> Unfortunately, the type of the implicit object parameter is suddenly
> relevant for cases like this.
>
> Anyway, as I've stated a few times, I'm going to implement my function
> that checks correspondence of the object parameter of iobj and xobj
> member functions assuming that iobj member functions introduced by
> using declarations are handled properly. I think that's the best option
> for my patch right now.
>
> Well that investigation took the majority of my day. I'm just glad I'm
> certain of what direction to take now.
>
> Alex
>
>
> On Monday, January 1st, 2024 at 8:34 AM, waffl3x waff...@protonmail.com wrote:
>
>
>
> > That was faster than I expected, the problem is exactly just that we
> > aren't implementing [over.match.funcs.general.4] at all. The result of
> > compparms for the 2 functions is false which I believe to be wrong. I
> > believe we have a few choices here, but no matter what we go with it
> > will be a bit of an overhaul. I will post a PR on bugzilla in a little
> > bit as this problem feels somewhat out of the scope of my patch now.
> >
> > I think what I will do is instead of comparing the xobj parameter to
> > the DECL_CONTEXT of the xobj function, I will compare it to the type of
> > the iobj member function's object parameter. If I do it like this, it
> > will work as expected if we rewrite functions that are introduced with
> > a using declaration.
> >
> > This might still cause problems, I will look into how the this pointer
> > for iobj member functions is determined again. Depending on how it is
> > determined, it might be possible to change the function signature of
> > iobj member functions without altering their behavior. It would be
> > incorrect, and would change the meaning of code, if changing the
> > function signature changed the type of the this pointer.
> >
> > Anyhow, this is a fairly big change to consider so I won't pretend I
> > know what the right call is. But the way I've decided to implement
> > correspondence checking will be consistent with how GCC currently
> > (incorrectly) treats constraints on iobj member functions introduced
> > with a using declaration, so I think doing it this way is the right
> > choice for now.
> >
> > Some days feel really unproductive when the majority is investigation
> > and thinking. This was one of them, but at least I'm confident that my
> > conclusions are correct. Aren't edge cases fun?
> >
> > Alex
> >
> > On Monday, January 1st, 2024 at 8:17 AM, waffl3x waff...@protonmail.com
> > wrote:
> >
> > > I've been at this for a while, and I'm not sure what the proper way to
> > > fix this is.
> > >
> > > `struct S; struct B { void f(this S&) {} void g() {} }; struct S : B {
> > > using B::f; using B::g; void f() {} void g(this S&) {} }; int main() { S
> > > s{}; s.f(); s.g(); }`
> > >
> > > In short, the call to f is
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113191
I've posted the report here, I ended up doing a bit more investigation,
so the contents might interest you. I'm really starting to think that
we want to do a more thorough re-engineering of how using declarations
are handled, instead of handling it with little hacks like the one
added to more_specialized_fn.
As I note in the report, the addition of xobj member functions really
makes [over.match.funcs.general.4] a lot more relevant, and I don't
think we can get away with not following it more closely anymore. I
know I'm wording myself here as if that passage has existed forever,
but I recognize it might be as recent as C++23 that it was added. I
don't mean to imply anything with how I'm wording it, it's just way
easier to express it this way. Especially since we really could get
away with these kinds of hacks if xobj member functions did not exist.
Unfortunately, the type of the implicit object parameter is suddenly
relevant for cases like this.
Anyway, as I've stated a few times, I'm going to implement my function
that checks correspondence of the object parameter of iobj and xobj
member functions assuming that iobj member functions introduced by
using declarations are handled properly. I think that's the best option
for my patch right now.
Well that investigation took the majority of my day. I'm just glad I'm
certain of what direction to take now.
Alex
On Monday, January 1st, 2024 at 8:34 AM, waffl3x wrote:
>
>
> That was faster than I expected, the problem is exactly just that we
> aren't implementing [over.match.funcs.general.4] at all. The result of
> compparms for the 2 functions is false which I believe to be wrong. I
> believe we have a few choices here, but no matter what we go with it
> will be a bit of an overhaul. I will post a PR on bugzilla in a little
> bit as this problem feels somewhat out of the scope of my patch now.
>
> I think what I will do is instead of comparing the xobj parameter to
> the DECL_CONTEXT of the xobj function, I will compare it to the type of
> the iobj member function's object parameter. If I do it like this, it
> will work as expected if we rewrite functions that are introduced with
> a using declaration.
>
> This might still cause problems, I will look into how the this pointer
> for iobj member functions is determined again. Depending on how it is
> determined, it might be possible to change the function signature of
> iobj member functions without altering their behavior. It would be
> incorrect, and would change the meaning of code, if changing the
> function signature changed the type of the this pointer.
>
> Anyhow, this is a fairly big change to consider so I won't pretend I
> know what the right call is. But the way I've decided to implement
> correspondence checking will be consistent with how GCC currently
> (incorrectly) treats constraints on iobj member functions introduced
> with a using declaration, so I think doing it this way is the right
> choice for now.
>
> Some days feel really unproductive when the majority is investigation
> and thinking. This was one of them, but at least I'm confident that my
> conclusions are correct. Aren't edge cases fun?
>
> Alex
>
> On Monday, January 1st, 2024 at 8:17 AM, waffl3x waff...@protonmail.com wrote:
>
>
>
> > I've been at this for a while, and I'm not sure what the proper way to
> > fix this is.
> >
> > `struct S; struct B { void f(this S&) {} void g() {} }; struct S : B {
> > using B::f; using B::g; void f() {} void g(this S&) {} }; int main() { S
> > s{}; s.f(); s.g(); }`
> >
> > In short, the call to f is ambiguous, but the call to g is not. I
> > already know where the problem is, but since I share this code in
> > places that don't know about whether a function was introduced by a
> > using declaration (cand_parms_match), I don't want to rely on that to
> > solve the problem.
> >
> > `/* An iobj member function's object parameter can't be an unrelated type,
> > if the xobj member function's object parameter is an unrelated type we know
> > they must not correspond to each other. If the iobj member function was
> > introduced with a using declaration, then the type of its object parameter
> > is still that of the class we are currently adding a member function to, so
> > this assumption holds true in that case as well.
> > [over.match.funcs.general.4] For non-conversion functions that are implicit
> > object member functions nominated by a using-declaration in a derived
> > class, the function is considered to be a member of the derived class for
> > the purpose of defining the type of the implicit object parameter. We don't
> > get to bail yet out even if
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
That was faster than I expected, the problem is exactly just that we
aren't implementing [over.match.funcs.general.4] at all. The result of
compparms for the 2 functions is false which I believe to be wrong. I
believe we have a few choices here, but no matter what we go with it
will be a bit of an overhaul. I will post a PR on bugzilla in a little
bit as this problem feels somewhat out of the scope of my patch now.
I think what I will do is instead of comparing the xobj parameter to
the DECL_CONTEXT of the xobj function, I will compare it to the type of
the iobj member function's object parameter. If I do it like this, it
will work as expected if we rewrite functions that are introduced with
a using declaration.
This might still cause problems, I will look into how the this pointer
for iobj member functions is determined again. Depending on how it is
determined, it might be possible to change the function signature of
iobj member functions without altering their behavior. It would be
incorrect, and would change the meaning of code, if changing the
function signature changed the type of the this pointer.
Anyhow, this is a fairly big change to consider so I won't pretend I
know what the right call is. But the way I've decided to implement
correspondence checking will be consistent with how GCC currently
(incorrectly) treats constraints on iobj member functions introduced
with a using declaration, so I think doing it this way is the right
choice for now.
Some days feel really unproductive when the majority is investigation
and thinking. This was one of them, but at least I'm confident that my
conclusions are correct. Aren't edge cases fun?
Alex
On Monday, January 1st, 2024 at 8:17 AM, waffl3x wrote:
>
>
> I've been at this for a while, and I'm not sure what the proper way to
> fix this is.
>
> `struct S; struct B { void f(this S&) {} void g() {} }; struct S : B { using
> B::f; using B::g; void f() {} void g(this S&) {} }; int main() { S s{};
> s.f(); s.g(); }`
>
> In short, the call to f is ambiguous, but the call to g is not. I
> already know where the problem is, but since I share this code in
> places that don't know about whether a function was introduced by a
> using declaration (cand_parms_match), I don't want to rely on that to
> solve the problem.
>
> `/* An iobj member function's object parameter can't be an unrelated type, if
> the xobj member function's object parameter is an unrelated type we know they
> must not correspond to each other. If the iobj member function was introduced
> with a using declaration, then the type of its object parameter is still that
> of the class we are currently adding a member function to, so this assumption
> holds true in that case as well. [over.match.funcs.general.4] For
> non-conversion functions that are implicit object member functions nominated
> by a using-declaration in a derived class, the function is considered to be a
> member of the derived class for the purpose of defining the type of the
> implicit object parameter. We don't get to bail yet out even if the xobj
> parameter is by-value as elaborated on below. This also implicitly handles
> xobj parameters of type pointer. */ if (DECL_CONTEXT (xobj_fn) !=
> TYPE_MAIN_VARIANT (non_reference (xobj_param))) return false;`
>
> I feel like what we are actually supposed to be doing to be to the
> letter of the standard is to be creating a new function entirely, with
> a decl_context of the original class, which sounds omega silly, and
> might bring a new set of problems.
>
> I think I might have came up with an unfortunately fairly convoluted
> way to solve this just now, but I don't know if it brings another new
> set of problems. The assumptions I had when I originally implemented
> this in add_method bled through when I broke it out into it's own
> function. At the very least I need to better document how the function
> is intended to be used, at worst I'll need to consider whether it makes
> sense to be reusing this logic if the use cases are subtly different.
>
> I don't think the latter is the case now though, I'm noticing GCC just
> has a bug in general with constraints and using declarations.
>
> https://godbolt.org/z/EbGvjfG7E
>
> So it might actually just be better to be rewriting functions that are
> introduced by using declarations, I have a feeling that will be what
> introduces the least pain.
>
> I'm not sure where exactly GCC is deciding that a function introduced
> by a using declaration is different from an otherwise corresponding one
> declared directly in that class, but I have a feeling on where it is.
> Obviously it's in joust, but I'm not sure the object parameters are
> actually being compared.
>
> I'll investigate this bug and get back to you, I imagine fixing it is
&
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
I've been at this for a while, and I'm not sure what the proper way to
fix this is.
```
struct S;
struct B {
void f(this S&) {}
void g() {}
};
struct S : B {
using B::f;
using B::g;
void f() {}
void g(this S&) {}
};
int main()
{
S s{};
s.f();
s.g();
}
```
In short, the call to f is ambiguous, but the call to g is not. I
already know where the problem is, but since I share this code in
places that don't know about whether a function was introduced by a
using declaration (cand_parms_match), I don't want to rely on that to
solve the problem.
```
/* An iobj member function's object parameter can't be an unrelated type, if
the xobj member function's object parameter is an unrelated type we know
they must not correspond to each other. If the iobj member function was
introduced with a using declaration, then the type of its object parameter
is still that of the class we are currently adding a member function to,
so this assumption holds true in that case as well.
[over.match.funcs.general.4]
For non-conversion functions that are implicit object member
functions nominated by a using-declaration in a derived class, the
function is considered to be a member of the derived class for the purpose
of defining the type of the implicit object parameter.
We don't get to bail yet out even if the xobj parameter is by-value as
elaborated on below.
This also implicitly handles xobj parameters of type pointer. */
if (DECL_CONTEXT (xobj_fn) != TYPE_MAIN_VARIANT (non_reference (xobj_param)))
return false;
```
I feel like what we are actually supposed to be doing to be to the
letter of the standard is to be creating a new function entirely, with
a decl_context of the original class, which sounds omega silly, and
might bring a new set of problems.
I think I might have came up with an unfortunately fairly convoluted
way to solve this just now, but I don't know if it brings another new
set of problems. The assumptions I had when I originally implemented
this in add_method bled through when I broke it out into it's own
function. At the very least I need to better document how the function
is intended to be used, at worst I'll need to consider whether it makes
sense to be reusing this logic if the use cases are subtly different.
I don't think the latter is the case now though, I'm noticing GCC just
has a bug in general with constraints and using declarations.
https://godbolt.org/z/EbGvjfG7E
So it might actually just be better to be rewriting functions that are
introduced by using declarations, I have a feeling that will be what
introduces the least pain.
I'm not sure where exactly GCC is deciding that a function introduced
by a using declaration is different from an otherwise corresponding one
declared directly in that class, but I have a feeling on where it is.
Obviously it's in joust, but I'm not sure the object parameters are
actually being compared.
I'll investigate this bug and get back to you, I imagine fixing it is
going to be key to actually implementing the xobj case without hacks.
Finding both these issues has slowed down my next revision as I noticed
the problem while cleaning up my implementation of CWG2789. I want to
note, I am implementing it as if it specifies corresponding object
arguments, not object arguments of the same type, as we previously
discussed, I believe that to be the right resolution as there are
really bad edge cases with the current wording.
Alex
On Tuesday, December 26th, 2023 at 9:37 AM, Jason Merrill
wrote:
>
>
> On 12/23/23 02:10, waffl3x wrote:
>
> > On Friday, December 22nd, 2023 at 10:26 AM, Jason Merrill ja...@redhat.com
> > wrote:
> >
> > > On 12/22/23 04:01, waffl3x wrote:
> > >
> > > > int n = 0;
> > > > auto f = [](this Self){
> > > > static_assert(__is_same (decltype(n), int));
> > > > decltype((n)) a; // { dg-error {is not captured} }
> > > > };
> > > > f();
> > > >
> > > > Could you clarify if this error being removed was intentional. I do
> > > > recall that Patrick Palka wanted to remove this error in his patch, but
> > > > it seemed to me like you stated it would be incorrect to allow it.
> > > > Since the error is no longer present I assume I am misunderstanding the
> > > > exchange.
> > > >
> > > > In any case, let me know if I need to modify my test case or if this
> > > > error needs to be added back in.
> > >
> > > Removing the error was correct under
> > > https://eel.is/c++draft/expr.prim#id.unqual-3
> > > Naming n in that lambda would not refer a capture by copy, so the
> > > decltype is the same as outside the lambda.
> >
> > Alright, I've fixed my tests to ref
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Friday, December 22nd, 2023 at 10:26 AM, Jason Merrill
wrote:
>
>
> On 12/22/23 04:01, waffl3x wrote:
>
> > int n = 0;
> > auto f = [](this Self){
> > static_assert(__is_same (decltype(n), int));
> > decltype((n)) a; // { dg-error {is not captured} }
> > };
> > f();
> >
> > Could you clarify if this error being removed was intentional. I do
> > recall that Patrick Palka wanted to remove this error in his patch, but
> > it seemed to me like you stated it would be incorrect to allow it.
> > Since the error is no longer present I assume I am misunderstanding the
> > exchange.
> >
> > In any case, let me know if I need to modify my test case or if this
> > error needs to be added back in.
>
>
> Removing the error was correct under
> https://eel.is/c++draft/expr.prim#id.unqual-3
> Naming n in that lambda would not refer a capture by copy, so the
> decltype is the same as outside the lambda.
>
> Jason
Alright, I've fixed my tests to reflect that.
I've got defaulting assignment operators working. Defaulting equality
and comparison operators seemed to work out of the box somehow, so I
just have to make some fleshed out tests for those cases.
There can always be more tests, I have a few ideas for what still needs
to be covered, mostly with dependent lambdas. Tests for xobj conversion
operators definitely need to be more fleshed out. I also need to
formulate some tests to make sure constraints are not being taking into
account when the object parameters should not correspond, but that's a
little more tough to test for than the valid cases.
Other than tests though, is there anything you can think of that the
patch is missing? Other than the aforementioned tests, I'm pretty
confident everything is done.
To recap, I have CWG2789 implemented on my end with the change we
discussed to require corresponding object parameters instead of the
same type, and I have CWG2586 implemented. I can't recall what other
outstanding issues we had, and my notes don't mention anything other
than tests. So I'm assuming everything is good.
Alex
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
int n = 0;
auto f = [](this Self){
static_assert(__is_same (decltype(n), int));
decltype((n)) a; // { dg-error {is not captured} }
};
f();
Could you clarify if this error being removed was intentional. I do
recall that Patrick Palka wanted to remove this error in his patch, but
it seemed to me like you stated it would be incorrect to allow it.
Since the error is no longer present I assume I am misunderstanding the
exchange.
In any case, let me know if I need to modify my test case or if this
error needs to be added back in.
Alex
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Friday, December 8th, 2023 at 12:25 PM, Jason Merrill
wrote:
>
>
> On 12/6/23 02:33, waffl3x wrote:
>
> > Here is the next version, it feels very close to finished. As before, I
> > haven't ran a bootstrap or the full testsuite yet but I did run the
> > explicit-obj tests which completed as expected.
> >
> > There's a few test cases that still need to be written but more tests
> > can always be added. The behavior added by CWG2789 works in at least
> > one case, but I have not added tests for it yet. The test cases for
> > dependent lambda expressions need to be fleshed out more, but a few
> > temporary ones are included to demonstrate that they do work and that
> > the crash is fixed. Explicit object conversion functions work, but I
> > need to add fleshed out tests for them, explicit-obj-basic5.C has that
> > test.
>
> > @@ -6586,6 +6586,17 @@ add_candidates (tree fns, tree first_arg, const
> > vec args,
> > + / FIXME: I believe this will be bugged for xobj member functions,
> > + leaving this comment here to make sure we look into it
> > + at some point.
> > + Seeing this makes me want correspondence checking to be unified
> > + in one place though, not sure if this one needs to be different
> > + from other ones though.
> > + This function is only used here, but maybe we can use it in add
> > + method and move some of the logic out of there?
>
>
> fns_correspond absolutely needs updating to handle xob fns, and doing
> that by unifying it with add_method's calculation would be good.
>
> > + Side note: CWG2586 might be relevant for this area in
> > + particular, perhaps we wait to see if it gets accepted first? */
>
>
> 2586 was accepted last year.
>
> > @@ -12574,17 +12601,25 @@ cand_parms_match (z_candidate *c1, z_candidate c2)
> > fn1 = DECL_TEMPLATE_RESULT (t1);
> > fn2 = DECL_TEMPLATE_RESULT (t2);
> > }
> > + / The changes I made here might be stuff I was told not to worry about?
> > + I'm not really sure so I'm going to leave it in. */
>
>
> Good choice, this comment can go.
>
> > tree parms1 = TYPE_ARG_TYPES (TREE_TYPE (fn1));
> > tree parms2 = TYPE_ARG_TYPES (TREE_TYPE (fn2));
> > if (DECL_FUNCTION_MEMBER_P (fn1)
> > && DECL_FUNCTION_MEMBER_P (fn2)
> > - && (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn1)
> > - != DECL_NONSTATIC_MEMBER_FUNCTION_P (fn2)))
> > + && (DECL_STATIC_FUNCTION_P (fn1)
> > + != DECL_STATIC_FUNCTION_P (fn2)))
> > {
> > /* Ignore 'this' when comparing the parameters of a static member
> > function with those of a non-static one. */
> > - parms1 = skip_artificial_parms_for (fn1, parms1);
> > - parms2 = skip_artificial_parms_for (fn2, parms2);
> > + auto skip_parms = [](tree fn, tree parms){
> > + if (DECL_XOBJ_MEMBER_FUNCTION_P (fn))
> > + return TREE_CHAIN (parms);
> > + else
> > + return skip_artificial_parms_for (fn, parms);
> > + };
> > + parms1 = skip_parms (fn1, parms1);
> > + parms2 = skip_parms (fn2, parms2);
> > }
>
>
> https://cplusplus.github.io/CWG/issues/2789.html fixes the handling of
> xobj fns here.
I have a minor concern here.
https://eel.is/c++draft/basic.scope.scope#3
Is it okay that CWG2789 has separate rules than the rules for
declarations? As far as I know there's nothing else that describes how
to handle the object parameters so it was my assumption that the rules
here are also used for that. I know at least one person has disagreed
with me about that so I'm not sure what is actually correct.
template
struct S {
constexpr void f() {} // #f1
constexpr void f(this S&&) requires true {} // #f2
constexpr void g() requires true {} // #g1
constexpr void g(this S&&) {} // #g2
};
void test() {
S<>{}.f(); // calls #?
S<>{}.g(); // calls #?
}
But with the wording proposed by CWG2789, wouldn't this example would
be a problem? If we follow the standard to the letter, constraints
can't be applied here right?
I wouldn't be surprised if I'm missing something but I figured I ought
to raise it just in case. Obviously it should call #f2 and #g1 but I'm
pretty sure the current wording only allows these calls to be ambiguous.
> Your change does the right thing for comparing static and xobj, but
> doesn't handle comparing iobj and xobj; I think we want to share
> parameter comparison code with fns_correspond/add_method. Maybe
> parms_correspond?
Yeah, I'll see what I can put together. The only issue being what I
noted above, I'm not sure the rules are actually the same. I think they
should be, but my reading of things seems like it'
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Follow up to this, bootstrapped and tested with no regressions. On Wednesday, December 6th, 2023 at 12:33 AM, waffl3x wrote: > > > Here is the next version, it feels very close to finished. As before, I > haven't ran a bootstrap or the full testsuite yet but I did run the > explicit-obj tests which completed as expected. > > There's a few test cases that still need to be written but more tests > can always be added. The behavior added by CWG2789 works in at least > one case, but I have not added tests for it yet. The test cases for > dependent lambda expressions need to be fleshed out more, but a few > temporary ones are included to demonstrate that they do work and that > the crash is fixed. Explicit object conversion functions work, but I > need to add fleshed out tests for them, explicit-obj-basic5.C has that > test. > > I'll start the tests now and report back if anything fails, I'm > confident everything will be fine though. > > Alex
Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Wednesday, December 6th, 2023 at 1:48 AM, Jakub Jelinek
wrote:
>
>
> On Wed, Dec 06, 2023 at 07:33:21AM +, waffl3x wrote:
>
> > Here is the next version, it feels very close to finished. As before, I
> > haven't ran a bootstrap or the full testsuite yet but I did run the
> > explicit-obj tests which completed as expected.
> >
> > There's a few test cases that still need to be written but more tests
> > can always be added. The behavior added by CWG2789 works in at least
> > one case, but I have not added tests for it yet. The test cases for
> > dependent lambda expressions need to be fleshed out more, but a few
> > temporary ones are included to demonstrate that they do work and that
> > the crash is fixed. Explicit object conversion functions work, but I
> > need to add fleshed out tests for them, explicit-obj-basic5.C has that
> > test.
> >
> > I'll start the tests now and report back if anything fails, I'm
> > confident everything will be fine though.
> >
> > Alex
>
> > From 937e12c57145bfd878a0bc4cd9735c2d3c4fcf22 Mon Sep 17 00:00:00 2001
> > From: Waffl3x waff...@protonmail.com
> > Date: Tue, 5 Dec 2023 23:16:01 -0700
> > Subject: [PATCH] P0847R7 (Deducing This) [PR102609] Another quick and dirty
> > patch for review, hopefully the last. gcc/cp/ChangeLog:
>
>
> Please add
> PR c++/102609
> line above this.
>
> > * call.cc (build_this_conversion):
>
>
> Note, for the final submission, all the ):
> should be followed by descriptions what has changed in there (but not why).
Yeah, I remember the drill, it just takes me a long time so I've been
slacking.
> Plus it would be good to mention somewhere early in the cp/ChangeLog
> entry that the patch implements C++23 P0847R7 - Deducing this paper
> (unfortunately the ChangeLog verifier doesn't allow such free text above
> the ChangeLog entry where it used to be written some years ago,
> only allows there the PR line; I usually put such text after the ):
> of the first entry now and only after it write exactly what changed
> in that function. Does the patch also implement CWG2586?
Oh jeez, I had been doing it the way you're saying is rejected.
Shouldn't the ChangeLog verifier be changed to allow this?
The patch does not implement CWG2586 at this time. I couldn't determine
if it were ready to go or not. I have a skeleton of tests for it that I
never finished, but as far as I know the implementation does conform to
CWG2789, this just happened to be how it worked out.
>
> Also, I don't see in the patch the expected
> gcc/c-family/
> * c-cppbuiltin.cc (c_cpp_builtins): Predefine
> __cpp_explicit_this_parameter=202110L for C++23.
> part plus gcc/testsuite/cpp{23,26}/feat-cxx*.C additions checking
> for that macro presence and its value.
>
> Jakub
Yeah I was meaning to look into how to do that, I originally added the
test and then never included it in any of the patches, or that's what
remember anyway. This saves me the work though, I'll be sure to add
that.
Alex
Re: [PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]
On Tuesday, December 5th, 2023 at 9:36 PM, Jason Merrill
wrote:
>
>
> On 12/5/23 23:23, waffl3x wrote:
>
> > Does CWG2834 effect this weird edge case?
>
>
> 2834 affects all partial ordering with explicit object member functions;
Both in relation to each other, and to iobj and static member functions?
> currently the working draft says that they get an additional fake object
> parameter, which is clearly wrong.
Yeah, that's really weird. I was under the impression that's how static
member functions worked, I didn't realize it was also how it's
specified for xobj member functions. I still find it weird for static
member functions. I guess I'll have to study template partial ordering,
what it is, how it's specified and whatnot. I think I understand it
intuitively but not at a language law level.
> > I couldn't quite grasp the
> > standardese so I'm not really sure. These are a few cases from a test
> > that I finalized last night. I ran this by jwakely and he agreed that
> > the behavior as shown is correct by the standard. I'll also add that
> > this is also the current behavior of my patch.
> >
> > template concept Constrain = true;
> >
> > inline constexpr int iobj_fn = 5;
> > inline constexpr int xobj_fn = 10;
> >
> > struct S {
> > int f(Constrain auto) { return iobj_fn; };
> > int f(this S&&, auto) { return xobj_fn; };
> >
> > int g(auto) { return iobj_fn; };
> > int g(this S&&, Constrain auto) { return xobj_fn; };
> > };
> > int main() {
> > S s{};
> > s.f (0) == iobj_fn;
>
>
> Yes, the xobj fn isn't viable because it takes an rvalue ref.
>
> > static_cast(s).f (0) == iobj_fn;
>
>
> Yes, the functions look the same to partial ordering, so we compare
> constraints and the iobj fn is more constrained.
>
> > s.g (0) == iobj_fn;
>
>
> Yes, the xobj fn isn't viable.
>
> > static_cast(s).g (0) == xobj_fn;
>
>
> Yes, the xobj fn is more constrained.
>
> Jason
It's funny to see you effortlessly agree with what took me a number of
hours pondering.
So just to confirm, you're also saying the changes proposed by CWG2834
will not change the behavior of this example?
Alex
Re: [PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]
Does CWG2834 effect this weird edge case? I couldn't quite grasp the
standardese so I'm not really sure. These are a few cases from a test
that I finalized last night. I ran this by jwakely and he agreed that
the behavior as shown is correct by the standard. I'll also add that
this is also the current behavior of my patch.
template concept Constrain = true;
inline constexpr int iobj_fn = 5;
inline constexpr int xobj_fn = 10;
struct S {
int f(Constrain auto) { return iobj_fn; };
int f(this S&&, auto) { return xobj_fn; };
int g(auto) { return iobj_fn; };
int g(this S&&, Constrain auto) { return xobj_fn; };
};
int main() {
S s{};
s.f (0) == iobj_fn;
static_cast(s).f (0) == iobj_fn;
s.g (0) == iobj_fn;
static_cast(s).g (0) == xobj_fn;
}
On Tuesday, December 5th, 2023 at 7:21 PM, Jason Merrill
wrote:
>
>
> Tested x86_64-pc-linux-gnu. Are the library test changes OK? A reduced
> example of the issue is at https://godbolt.org/z/cPxrcnKjG
>
> -- 8< --
>
> Looks like we implemented option 1 (skip the object parameter) for CWG532
> before the issue was resolved, and never updated to the final resolution of
> option 2 (model it as a reference). More recently CWG2445 extended this
> handling to static member functions; I think that's wrong, and have
> opened CWG2834 to address that and how explicit object member functions
> interact with it.
>
> The FIXME comments are to guide how the explicit object member function
> support should change the uses of DECL_NONSTATIC_MEMBER_FUNCTION_P.
>
> The library testsuite changes are to make partial ordering work again
> between the generic operator- in the testcase and
> _Pointer_adapter::operator-.
>
> DR 532
> PR c++/53499
>
> gcc/cp/ChangeLog:
>
> * pt.cc (more_specialized_fn): Fix object parameter handling.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/template/partial-order4.C: New test.
> * g++.dg/template/spec26.C: Adjust for CWG532.
>
> libstdc++-v3/ChangeLog:
>
> * testsuite/23_containers/vector/ext_pointer/types/1.cc
> * testsuite/23_containers/vector/ext_pointer/types/2.cc
> (N::operator-): Make less specialized.
> ---
> gcc/cp/pt.cc | 68 ++-
> .../g++.dg/template/partial-order4.C | 17 +
> gcc/testsuite/g++.dg/template/spec26.C | 10 +--
> .../vector/ext_pointer/types/1.cc | 4 +-
> .../vector/ext_pointer/types/2.cc | 4 +-
> 5 files changed, 78 insertions(+), 25 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/template/partial-order4.C
>
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 924a20973b4..4b2af4f7aca 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -25218,27 +25218,61 @@ more_specialized_fn (tree pat1, tree pat2, int len)
> bool lose1 = false;
> bool lose2 = false;
>
> - /* Remove the this parameter from non-static member functions. If
> - one is a non-static member function and the other is not a static
> - member function, remove the first parameter from that function
> - also. This situation occurs for operator functions where we
> - locate both a member function (with this pointer) and non-member
> - operator (with explicit first operand). /
> - if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1))
> + / C++17 [temp.func.order]/3 (CWG532)
> +
> + If only one of the function templates M is a non-static member of some
> + class A, M is considered to have a new first parameter inserted in its
> + function parameter list. Given cv as the cv-qualifiers of M (if any), the
> + new parameter is of type "rvalue reference to cv A" if the optional
> + ref-qualifier of M is && or if M has no ref-qualifier and the first
> + parameter of the other template has rvalue reference type. Otherwise, the
> + new parameter is of type "lvalue reference to cv A". /
> +
> + if (DECL_STATIC_FUNCTION_P (decl1) || DECL_STATIC_FUNCTION_P (decl2))
> {
> - len--; / LEN is the number of significant arguments for DECL1 /
> - args1 = TREE_CHAIN (args1);
> - if (!DECL_STATIC_FUNCTION_P (decl2))
> - args2 = TREE_CHAIN (args2);
> - }
> - else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> - {
> - args2 = TREE_CHAIN (args2);
> - if (!DECL_STATIC_FUNCTION_P (decl1))
> + / Note C++20 DR2445 extended the above to static member functions, but
> + I think think the old G++ behavior of just skipping the object
> + parameter when comparing to a static member function was better, so
> + let's stick with that for now. This is CWG2834. --jason 2023-12 /
> + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1)) / FIXME or explicit /
> {
> - len--;
> + len--; / LEN is the number of significant arguments for DECL1 /
> args1 = TREE_CHAIN (args1);
> }
> + else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2)) / FIXME or explicit /
> + args2 = TREE_CHAIN (args2);
> + }
> + else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1) / FIXME implicit only /
> + && DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> + {
> + / Note DR2445 also (IMO wrongly) removed the "only one" above, which
> + would break e.g.
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
>> @@ -15402,6 +15450,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
>> complain,
>> gcc_checking_assert (TYPE_MAIN_VARIANT (TREE_TYPE (ve))
>>== TYPE_MAIN_VARIANT (type));
>> SET_DECL_VALUE_EXPR (r, ve);
>> + if (is_capture_proxy (t))
>> + type = TREE_TYPE (ve);
>That should have close to the same effect as the lambda_proxy_type
>adjustment I was talking about, since that function basically returns
>the TREE_TYPE of the COMPONENT_REF. But the underlying problem is that
>finish_non_static_data_member assumes that 'object' is '*this', for
>which you can trust the cv-quals; for auto&&, you can't.
>capture_decltype has the same problem. I'm attaching a patch to address
>this in both places.
Regarding this, was my change actually okay, and was your change
supposed to address it? I applied my patch to the latest commit in
master yesterday and started tests and whatnot with this change
commented out as I wasn't sure. It seems like my tests for constness of
captures no longer works with or without this change commented out.
If you wish I can go over everything again and figure out a new
solution with your changes but stepping through all this code was quite
a task that I'm weary of doing again. Even if the second time through
won't be so arduous I would like to avoid it.
You know what, I'll give it a go anyway but I don't want to spend too
much time on it, I still have a few tests to clean up and this crash to
fix.
template void f()
{
int i;
[=](this T&& self){ return i; }(); // error, unrelated
}
int main() { f(); }
If this crash doesn't take too long (I don't think it will, it seems
straightforward enough) then I'll look at fixing the captures with a
const xobject parameter bug the correct way.
Alex
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Monday, December 4th, 2023 at 9:35 PM, waffl3x
wrote:
>
>
> >> @@ -15402,6 +15450,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
> >> complain,
>
> > > gcc_checking_assert (TYPE_MAIN_VARIANT (TREE_TYPE (ve))
> > > == TYPE_MAIN_VARIANT (type));
> > > SET_DECL_VALUE_EXPR (r, ve);
> > > + if (is_capture_proxy (t))
> > > + type = TREE_TYPE (ve);
>
> > That should have close to the same effect as the lambda_proxy_type
> > adjustment I was talking about, since that function basically returns
> > the TREE_TYPE of the COMPONENT_REF. But the underlying problem is that
> > finish_non_static_data_member assumes that 'object' is '*this', for
> > which you can trust the cv-quals; for auto&&, you can't.
> > capture_decltype has the same problem. I'm attaching a patch to address
> > this in both places.
>
>
> Regarding this, was my change actually okay, and was your change
> supposed to address it? I applied my patch to the latest commit in
> master yesterday and started tests and whatnot with this change
> commented out as I wasn't sure. It seems like my tests for constness of
> captures no longer works with or without this change commented out.
>
> If you wish I can go over everything again and figure out a new
> solution with your changes but stepping through all this code was quite
> a task that I'm weary of doing again. Even if the second time through
> won't be so arduous I would like to avoid it.
>
> You know what, I'll give it a go anyway but I don't want to spend too
> much time on it, I still have a few tests to clean up and this crash to
> fix.
>
> template void f()
>
> {
> int i;
> [=](this T&& self){ return i; }(); // error, unrelated
> }
> int main() { f(); }
>
>
> If this crash doesn't take too long (I don't think it will, it seems
> straightforward enough) then I'll look at fixing the captures with a
> const xobject parameter bug the correct way.
>
> Alex
WAIT Scratch that, I made a mistake, there's only a single case that is
broken, I read the test log wrong. Ah, I swear I'm cursed to realize
things the moment I hit the send button.
I have to take a closer look, I'll get back to you when I know more,
just trying to make sure you don't waste your time on this due to my
mistake.
Alex
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Monday, December 4th, 2023 at 9:39 PM, waffl3x
wrote:
> On Monday, December 4th, 2023 at 9:35 PM, waffl3x waff...@protonmail.com
> wrote:
>
>
>
> > > > @@ -15402,6 +15450,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
> > > > complain,
> >
> > > > gcc_checking_assert (TYPE_MAIN_VARIANT (TREE_TYPE (ve))
> > > > == TYPE_MAIN_VARIANT (type));
> > > > SET_DECL_VALUE_EXPR (r, ve);
> > > > + if (is_capture_proxy (t))
> > > > + type = TREE_TYPE (ve);
> >
> > > That should have close to the same effect as the lambda_proxy_type
> > > adjustment I was talking about, since that function basically returns
> > > the TREE_TYPE of the COMPONENT_REF. But the underlying problem is that
> > > finish_non_static_data_member assumes that 'object' is '*this', for
> > > which you can trust the cv-quals; for auto&&, you can't.
> > > capture_decltype has the same problem. I'm attaching a patch to address
> > > this in both places.
> >
> > Regarding this, was my change actually okay, and was your change
> > supposed to address it? I applied my patch to the latest commit in
> > master yesterday and started tests and whatnot with this change
> > commented out as I wasn't sure. It seems like my tests for constness of
> > captures no longer works with or without this change commented out.
> >
> > If you wish I can go over everything again and figure out a new
> > solution with your changes but stepping through all this code was quite
> > a task that I'm weary of doing again. Even if the second time through
> > won't be so arduous I would like to avoid it.
> >
> > You know what, I'll give it a go anyway but I don't want to spend too
> > much time on it, I still have a few tests to clean up and this crash to
> > fix.
> >
> > template void f()
> >
> > {
> > int i;
> > [=](this T&& self){ return i; }(); // error, unrelated
> > }
> > int main() { f(); }
> >
> > If this crash doesn't take too long (I don't think it will, it seems
> > straightforward enough) then I'll look at fixing the captures with a
> > const xobject parameter bug the correct way.
> >
> > Alex
>
>
> WAIT Scratch that, I made a mistake, there's only a single case that is
> broken, I read the test log wrong. Ah, I swear I'm cursed to realize
> things the moment I hit the send button.
>
> I have to take a closer look, I'll get back to you when I know more,
> just trying to make sure you don't waste your time on this due to my
> mistake.
>
> Alex
tl;dr it wasn't important, I just have to fix my test.
Okay that was faster than I anticipated, but unfortunately I don't know
how to handle it. I think your change in finish_non_static_data_member
might have been too heavy handed, but I don't know if there's a middle
ground. Or that's what I was going to say until I tested my assumption
on godbolt.
void f(auto const& a) { a = 5; }
Clang, MSVC and GCC all accept this until it is actually instantiated.
So, the true answer to my test failing is to just instantiate the
template. The test in question that was failing looks like this.
auto f2 = [n = 5](this auto const&){ n = 10; }; // { dg-error {} }
With the way things were before, this actually worked, so what my
assumption is now is that for us to actually diagnose this before a
template is instantiated would take some significant reworking of how
things are currently done. AND, I don't even know if it's legal for us
to make this diagnostic before instantiation for either of these cases.
Hah, come to think of it, we can't, there could be an overloaded
operator= that this is valid for... how disappointing.
We can for lambdas since the type is not dependent (on the lambda
instantiation) but it just isn't worth the effort I reckon.
Whatever, moving on, spending time on these things always drains me
because I think "oh boy I can do something better" and finding out it's
just not possible sucks. It's worse when it's because I overlooked
something that's obvious in hindsight.
Oh well, only that crash left I believe.
Alex
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Friday, December 1st, 2023 at 9:52 AM, Jason Merrill
wrote:
>
>
> On 12/1/23 01:02, waffl3x wrote:
>
> > I ran into another issue while devising tests for redeclarations of
> > xobj member functions as static member functions and vice versa. I am
> > pretty sure by the literal wording of the standard, this is well formed.
> >
> > template
> > concept Constrain = true;
> >
> > struct S {
> > void f(this auto, Constrain auto) {};
> > static void f(Constrain auto) {};
> >
> > void g(this auto const&, Constrain auto) {};
> > static void g(Constrain auto) {};
> >
> > void h(this auto&&, Constrain auto) {};
> > static void h(Constrain auto) {};
> > };
> >
> > And also,
> >
> > struct S{
> > void f(this auto) {};
> > static void f() {};
> >
> > void g(this auto const&) {};
> > static void g() {};
> >
> > void h(this auto&&) {};
> > static void h() {};
> > };
> >
> > I wrote these tests expecting them to be ill-formed, and found what I
> > thought was a bug when they were not diagnosed as redecelarations.
> > However, given how the code for resolving overloads and determining
> > redeclarations looks, I believe this is actually well formed on a
> > technicality. I can't find the passages in the standard that specify
> > this so I can't be sure.
>
>
> I think the relevant section is
> https://eel.is/c++draft/basic.scope.scope
>
> > Anyway, the template parameter list differs because of the deduced
> > object parameter. Now here is the question, you are required to ignore
> > the object parameter when determining if these are redeclarations or
> > not, but what about the template parameters associated with the object
> > parameter? Am I just missing the passage that specifies this or is this
> > an actual defect in the standard?
>
>
> I think that since they differ in template parameters, they don't
> correspond under https://eel.is/c++draft/basic.scope.scope#4.5 so they
> can be overloaded.
>
> This is specified in terms of the template-head grammar non-terminal,
> but elsewhere we say that abbreviated templates are equivalent to
> writing out the template parameters explicitly.
>
> > The annoying thing is, even if this was brought up, I think the only
> > solution is to ratify these examples as well formed.
>
>
> Yes.
>
> Jason
I can't get over that I feel like this goes against the spirit of the
specification. Just because an object argument is deduced should not
suddenly mean we take it into account. Too bad there's no good solution.
I especially don't like that that the following case is ambiguous. I
understand why, but I don't like it.
template
concept Constrain = true;
struct S {
int f(this auto, Constrain auto) {};
static f(auto) {};
};
main() {
S{}.f(0);
}
I would like to see this changed honestly. When an ambiguity is
encountered, the more constrained function should be taken into account
even if they normally can't be considered. Is there some pitfall with
this line of thinking that kept it out of the standard? Is it just a
case of "too hard to specify" or is there some reason it's impossible
to do in all but the simplest of cases?
Anyway while I do think this behavior is bad (not wrong according to
the standard, but bad imo), I recognize I don't have time to think
about it right now so I'll go back to working on the patch for the time
being.
Alex
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
I ran into another issue while devising tests for redeclarations of
xobj member functions as static member functions and vice versa. I am
pretty sure by the literal wording of the standard, this is well formed.
template
concept Constrain = true;
struct S {
void f(this auto, Constrain auto) {};
static void f(Constrain auto) {};
void g(this auto const&, Constrain auto) {};
static void g(Constrain auto) {};
void h(this auto&&, Constrain auto) {};
static void h(Constrain auto) {};
};
And also,
struct S{
void f(this auto) {};
static void f() {};
void g(this auto const&) {};
static void g() {};
void h(this auto&&) {};
static void h() {};
};
I wrote these tests expecting them to be ill-formed, and found what I
thought was a bug when they were not diagnosed as redecelarations.
However, given how the code for resolving overloads and determining
redeclarations looks, I believe this is actually well formed on a
technicality. I can't find the passages in the standard that specify
this so I can't be sure.
Anyway, the template parameter list differs because of the deduced
object parameter. Now here is the question, you are required to ignore
the object parameter when determining if these are redeclarations or
not, but what about the template parameters associated with the object
parameter? Am I just missing the passage that specifies this or is this
an actual defect in the standard?
The annoying thing is, even if this was brought up, I think the only
solution is to ratify these examples as well formed.
struct S {
template
void f(this T, T) {}
template
static void f(T) {}
};
Like what about this? If we ignore the template parameters associated
with the explicit object parameter, then the template parameter lists
don't match.
struct S {
template typename Templ, typename T>
void f(this Templ, T) {}
template
void f(T) {}
};
However, after writing them out and thinking about it a little, maybe
it really is just that simple. If after eliminating the template
parameters the explicit object parameter depends on the template
parameter lists are the same, then it's a redeclaration. Maybe this
works?
Am I overthinking this? Is there actually something specifying this
properly already? Hopefully that's the case but at the very least I
managed to write out this e-mail fairly quick for once so I didn't
waste too much time on this even if it does turn out to be nothing. The
majority of my time here was spent on the test case, which needed to be
written anyway.
Alex
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Wednesday, November 29th, 2023 at 10:00 PM, Jason Merrill
wrote:
>
>
> On 11/27/23 00:35, waffl3x wrote:
>
> > I think this is cleaned up enough to be presentable. Bootstrapped but
> > not tested but I don't think I changed anything substantial. I am
> > running tests right now and will report if anything fails. I have not
> > fixed the problem in tsubst_lambda_expr that we talked about, that will
> > be first on my list tomorrow. While writing/cleaning up tests I had to
> > investigate some things, one of which is calling an overloaded
> > function, where one of the candidates are introduced by a using
> > declaration, is considered ambiguous. I haven't narrowed down the case
> > for this yet so I don't know if it's related to xobj member
> > functions/lambda with xobj parameters or not. I had to pull a few tests
> > because of it though.
> >
> > I did not get as much done as I would have hoped today. This really
> > just serves as a small progress update. Once again, todo is the issue
> > you raised in tsubst_lambda_expr, and fixing handling of captures when
> > a const xobj parameter is deduced in a lamdba call operator.
>
> > +#define DECL_IOBJ_MEMBER_FUNC_P(NODE) \
> > +#define DECL_XOBJ_MEMBER_FUNC_P(NODE) \
> > +#define DECL_OBJECT_MEMBER_FUNC_P(NODE) \
>
>
> Let's use the full word FUNCTION in these macros for consistency with
> DECL_STATIC_FUNCTION_P.
Okay.
> > @@ -6544,7 +6544,7 @@ add_candidates (tree fns, tree first_arg, const
> > vec *args,
> > tree fn_first_arg = NULL_TREE;
> > const vec *fn_args = args;
> >
> > - if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn))
> > + if (DECL_OBJECT_MEMBER_FUNC_P (fn))
> > {
> > /* Figure out where the object arg comes from. If this
> > function is a non-static member and we didn't get an
>
>
> Hmm, that this function explicitly pulls out the object argument into
> first_arg strengthens your earlier argument that we shouldn't bother
> trying to handle null first_arg. But let's not mess with that in this
> patch.
Maybe I'll take some time to look into it when this patch is done, I
came across another block that seems to guarantee that first_arg gets
passed in a bit ago.
> > + val = handle_arg(TREE_VALUE (parm),
>
>
> Missing space.
Is there a script I can use for this so I'm not wasting your time on
little typos like this one?
> > /* We know an iobj parameter must be a reference. If our xobj
> > parameter is a pointer, we know this is not a redeclaration.
> > This also catches array parameters, those are pointers too. */
> > if (TYPE_PTR_P (xobj_param))
> > continue;
>
>
> Handling pointers specifically here seems unnecessary, they should be
> rare and will be handled by the next check for unrelated type.
Ah, it took me a second but I see it now, yeah I think I'll make this
change, with a comment that notes that it also handles the pointer case.
> > dealing with a by-value xobj parameter we can procede following
>
>
> "proceed"
>
> > /* An operator function must either be a non-static member function
> > or have at least one parameter of a class, a reference to a class,
> > an enumeration, or a reference to an enumeration. 13.4.0.6 /
> > - if (! methodp || DECL_STATIC_FUNCTION_P (decl))
> > + / This can just be DECL_STATIC_FUNCTION_P (decl) I think? */
> > + if ((!methodp && !DECL_XOBJ_MEMBER_FUNC_P (decl))
> > + || DECL_STATIC_FUNCTION_P (decl))
>
>
> No, it also needs to be true for non-members; rather, I think it can
> just be if (!DECL_OBJECT_FUNCTION_P (decl))
Yeah that seems to make sense, I'll try that.
> > + if (xobj_param)
> > + {
> > + quals = TYPE_UNQUALIFIED;
> > + if (TYPE_REF_P (xobj_param)
> > + && !(cp_type_quals (TREE_TYPE (xobj_param)) & TYPE_QUAL_CONST))
> > + LAMBDA_EXPR_MUTABLE_P (lambda_expr) = 1;
> > + }
>
>
> I don't think we want to mess with MUTABLE_P here. But then
> capture_decltype needs to be fixed to refer to the type of the object
> parameter rather than MUTABLE_P.
>
> Actually, I think we can do away with the MUTABLE_P flag entirely. I'm
> going to push a patch to do that.
I've been working on code that concerns it today and yesterday and I
was beginning to think the same thing. My current version doesn't set
it at all. I'm happy to see it removed.
When I looked into whether I should or should not be setting it I found
a new bug with decltype((capture)) in lambdas with default capture. I
am not going to try to fix the value capture default case but I was
able to fix the ref capture default case. Basically decltype((x)) is
not dependent right now a
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
This fixes the const bug. I haven't bootstrapped and tested beyond my own tests yet but this does it. I don't know if this is the right way to fix this yet, but I think it's pretty close. I'll see if I can make a better write up tomorrow, but it seems to me that since we never cared about substituting captures at function template instantiation time there was never a special case added in here to add the const over to the var_decl's type. I assume in cases where there is some kind of type mismatch that it's usually handled before here, but this special case throws a wrench into the works. @@ -15402,6 +15450,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain, gcc_checking_assert (TYPE_MAIN_VARIANT (TREE_TYPE (ve)) == TYPE_MAIN_VARIANT (type)); SET_DECL_VALUE_EXPR (r, ve); + if (is_capture_proxy (t)) + type = TREE_TYPE (ve); } if (CP_DECL_THREAD_LOCAL_P (r) && !processing_template_decl) Sigh, that was really tough to track down, but it's honestly pretty relieving that it was MOSTLY using the closure type and not totally ignoring it. It just misses a step here for some reason, idk if this is a larger bug that was benign before, or if this case just invalidates some assumptions, I'm not really sure. I don't really have the energy to fully understand everything that's going on in here tonight. This fix is just good enough to bootstrap and run the tests overnight and hopefully when I check in the morning there won't be any regressions. Going to get some rest now, as long as the other maybe bugs I found are not related to xobj parameters we can probably dust off this patch this week. Although, maybe I should have learned by now that I'm terrible at estimating timelines. Alex
Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Sunday, November 26th, 2023 at 7:40 PM, Jason Merrill
wrote:
>
>
> On 11/26/23 20:44, waffl3x wrote:
>
> > > > > > The other problem I'm having is
> > > > > >
> > > > > > auto f0 = [n = 5, ](this auto const&){ n = 10; };
> > > > > > This errors just fine, the lambda is unconditionally const so
> > > > > > LAMBDA_EXPR_MUTABLE_P flag is set for the closure.
> > > > > >
> > > > > > This on the other hand does not. The constness of the captures
> > > > > > depends
> > > > > > on (I assume) LAMBDA_EXPR_MUTABLE_P so all the captures are
> > > > > > non-const
> > > > > > here.
> > > > > > auto f1 = [n = 5](this auto&& self){ n = 10; };
> > > > > > as_const(f1)();
> > > > >
> > > > > That sounds correct, why would this be an error?
> > > > >
> > > > > The constness of the captures doesn't depend on LAMBDA_EXPR_MUTABLE_P,
> > > > > it depends on the type of the object parameter, which in this case is
> > > > > non-const, so so are the captures.
> > > >
> > > > Oops, I should have just made a less terse example, you missed the
> > > > "as_const" in the call to the lambda. The object parameter should be
> > > > deduced as const here. I definitely should have made that example
> > > > better, my bad.
> > >
> > > Ah, yes, I see it now.
> >
> > I don't remember if I relayed my planned fix for this to you. My
> > current idea is to modify the tree during instantiation of the lambda's
> > body somewhere near tsubst and apply const to all it's members. This is
> > unfortunately the best idea I have so far and it feels like an awful
> > hack. I am open to better ideas, but I don't think we can do anything
> > until the template is instantiated so I think it has to be there.
>
>
> I think the answer should be in lambda_proxy_type. The case where we
> build a DECLTYPE_TYPE may need to be expanded to cover this situation.
>
> > Should I wait until I fix the issue in tsubst_lambda_expr before
> > submitting the patch? I'm fine to do it either way, just whatever you
> > prefer. If I finish cleaning up these tests before I hear back I'll go
> > ahead and submit it and then start looking at different solutions in
> > there.
>
>
> Go ahead and submit.
>
> Jason
I'm going to need to sit down and set up a proper e-mail application
once I'm done with all this, I missed your reply because it went off to
another thread. Luckily, I decided to send the patch anyway, and when I
noticed that my patch was not under the same thread I came looking for
it. Ah well, what a pain, I guess getting used to these mail list
things is just going to take time.
> > It doesn't. The issue is messing with the type of (potentially) a lot
> > of different functions. Even if it doesn't actually break anything, it
> > seems like the kind of hidden mutation that you were objecting to.
>
>
> Oh... yeah..., I see the issue now. I still don't think the solution
> used for static lambdas will work, or be painless anyhow, but if I
> can't find something better I will try to use that one.
>
> > > Well, even so, I can just clear it after it gets used as we just need
> > > it to pass the closure type down. Perhaps I should have led with this,
> > > but as it stands the version that uses TYPE_METHOD_BASETYPE bootstraps
> > > with no regressions. I'll still look deeper but I'm pretty confident in
> > > my decision here, I really don't want to try to unravel what
> > > build_memfn_type does, I would rather find a whole different way of
> > > passing that information down.
> >
> > But the existing code already works fine, it's just a question of
> > changing the conditions so we handle xob lambdas the same way as static.
>
>
> I'm still concerned it wont cooperate with xobj parameters of unrelated
> type, but like I said, you've indicated my solution is definitely wrong
> so I'll look at fixing it.
I spent some time looking at it, I've decided you're probably right
that handling this the same way as the static lambda case is the best
in the short term. I still don't like it, but I've gone ahead and made
that change, and it seems to work just fine. I still find it icky, but
once I realized we do in fact need lambda_fntype since it might have
been substituted into in tsubst_lambda_expr, I don't see any better way
of doing this at the moment.
Since the added parameter just gets popped off by static_fn_type, and
tsubst_lambda_expr doesn't touch the xobj parameter, I'm pretty sure it
should behave properly. So no problems I guess, moving on to the
captures bug.
Alex
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> > > > OKAY, I figured out SOMETHING, I think this should be fine. As noted in
> > > > the comments, this might be a better way of handling the static lambda
> > > > case too. This is still a nasty hack so it should probably be done
> > > > differently, but I question if making a whole new fntype node in
> > > > tsubst_lambda_expr makes sense. On the other hand, maybe there will be
> > > > problems if a lambda is already instantiated? I'm not sure, mutating
> > > > things this way makes me uneasy though.
> > >
> > > A problem with changing TYPE_METHOD_BASETYPE for a FUNCTION_TYPE is that
> > > all equivalent FUNCTION_TYPE share the same tree node (through
> > > type_hash_canon), so you're messing with the type of unrelated functions
> > > at the same time. I think it's better to stick with the way static
> > > lambdas are handled.
> >
> > Yes, that was my concern as well, without even thinking about hashing.
> > I will investigate it deeper but I'm pretty sure TYPE_METHOD_BASETYPE,
> > while a valid field, is not used at all for function_type nodes. I had
> > to look into this when looking into the DECL_CONTEXT stuff previously,
> > so I'm pretty confident in this. Come to think of it, does hashing even
> > take fields that aren't "supposed" to be set for a node into account?
>
>
> It doesn't. The issue is messing with the type of (potentially) a lot
> of different functions. Even if it doesn't actually break anything, it
> seems like the kind of hidden mutation that you were objecting to.
Oh... yeah..., I see the issue now. I still don't think the solution
used for static lambdas will work, or be painless anyhow, but if I
can't find something better I will try to use that one.
> > Well, even so, I can just clear it after it gets used as we just need
> > it to pass the closure type down. Perhaps I should have led with this,
> > but as it stands the version that uses TYPE_METHOD_BASETYPE bootstraps
> > with no regressions. I'll still look deeper but I'm pretty confident in
> > my decision here, I really don't want to try to unravel what
> > build_memfn_type does, I would rather find a whole different way of
> > passing that information down.
>
>
> But the existing code already works fine, it's just a question of
> changing the conditions so we handle xob lambdas the same way as static.
I'm still concerned it wont cooperate with xobj parameters of unrelated
type, but like I said, you've indicated my solution is definitely wrong
so I'll look at fixing it.
> > Anyway, due to this, I am not currently concerned about the lack of a
> > diagnostic, and I believe my implementation is the most correct way to
> > do it. The problem with resolve_address_of_overloaded_function that I
> > go into below (depending on if you agree that it's a problem) is what
> > needs to be fixed to allow the current diagnostic to be properly
> > emitted. As of right now, there is no execution path that I know of
> > where the diagnostic will be properly emitted.
> >
> > I go into more detail about this in the comment in
> > tsubst_function_decl, although I do have to revise it a bit as I wrote
> > it before I had a discussion with another developer on the correct
> > behavior of the following case. I previously wondered if the overload
> > resolution from initializing p1 should result in a hard fault.
> >
> > template
> > struct S : T {
> > using T::operator();
> > void operator()(this int&, auto) {}
> > };
> >
> > int main()
> > {
> > auto s0 = S{[](this auto&&, auto){}};
> > // error, ambiguous
> > void (*p0)(int&, int) = (s0)::operator();
> >
> > auto s1 = S{[x = 42](this auto&&, auto){}};
> > // SFINAE's, calls S::operator()
> > void (*p1)(int&, int) = (s1)::operator();
> > }
> >
> > The wording in [expr.prim.lambda.closure-5] is similar to that in
> > [dcl.fct-15], and we harness SFINAE in that case, so he concluded that
> > this case (initializing p1) should also harness SFINAE.
>
>
> Agreed.
Perfect, glad we are on the same page there.
> > > > The other problem I'm having is
> > > >
> > > > auto f0 = [n = 5, ](this auto const&){ n = 10; };
> > > > This errors just fine, the lambda is unconditionally const so
> > > > LAMBDA_EXPR_MUTABLE_P flag is set for the closure.
> > > >
> > > > This on the other hand does not. The constness of the captures depends
> > > > on (I assume) LAMBDA_EXPR_MUTABLE_P so all the captures are non-const
> > > > here.
> > > > auto f1 = [n = 5](this auto&& self){ n = 10; };
> > > > as_const(f1)();
> > >
> > > That sounds correct, why would this be an error?
> > >
> > > The constness of the captures doesn't depend on LAMBDA_EXPR_MUTABLE_P,
> > > it depends on the type of the object parameter, which in this case is
> > > non-const, so so are the captures.
> >
> > Oops, I should have just made a less terse example, you missed the
> > "as_const" in the call to the lambda. The object parameter should be
> > deduced as const here. I definitely should have made that example
> > better, my
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> > Now with that out of the way, I do still kind of feel icky about it.
> > This really is a MASSIVE edge case that will almost never matter. As
> > far as I know, instantiating explicitly like so...
> >
> > auto f = [x = 42](this auto&&) -> int { return x; };
> > int (*fp)(int&) = (f)::operator();
> >
> > ...is the only way to coerce the explicit object parameter of the
> > lambda's call operator into deducing as an unrelated type. Cases that
> > are not deduced can be caught trivially while parsing the lambda and
> > are the only reasonable cases where you might have an unrelated type.
> > Perhaps it might become relevant in the future if a proposal like
> > https://atomgalaxy.github.io/isocpp-1107/D1107.html ever made it in,
> > but we aren't there yet. So as it stands, I'm pretty certain that it's
> > just impossible to instantiate a lambda's call operator with an
> > unrelated xobj parameter type except for the above case with
> > address-of. If you can think of another, please do share, but I've
> > spent a fair amount of time on it and came up with nothing.
>
>
> I think you're right.
>
I was about to send a quick e-mail amending this, before I respond to
everything else I want to include this test case I just came up with minutes
ago.
template
struct S : T {
using T::operator();
operator int() const {return {};}
};
int main()
{
auto s0 = S{[](this auto&& self) { return self; }};
auto s1 = S{[x = 0](this auto&& self) { return self; }};
s0.operator()();
s1.operator()();
}
So I was wrong, but, the good news is that this does demonstrate that there
is a code path where my diagnostic works.
template
concept NotInt = (!__is_same (T, int));
template struct enable_if {};
template<> struct enable_if { using type = decltype(nullptr); };
template using enable_if_t = typename enable_if::type;
template
void using_concepts(T) {}
template = nullptr>
void using_enable_if(T) {}
void test()
{
void (*fp_concepts)(int) = _concepts;
void (*fp_enable_if)(int) = _enable_if;
using_concepts(0);
using_enable_if(0);
}
I also have this test case that demonstrates the difference in diagnostic
quality. This is unrelated to explicit object member functions though, but
it does demonstrate that the diagnostics that I currently produce are in
equal quality to the ones already produced in these cases.
At this point I feel like I am unlikely to start fixing the bug with
captures not being treated as const tonight. Cleaning up the tests is taking
me longer than I thought.
Anyway I'm just rushing this e-mail to clarify this mistake, admittedly I am
a little excited to have found (which in hindsight should have been obvious)
a test case that more directly calls a lambda's call operator with an
unrelated type.
Alex
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Sunday, November 26th, 2023 at 2:30 PM, Jason Merrill
wrote:
>
>
> On 11/24/23 20:14, waffl3x wrote:
>
> > OKAY, I figured out SOMETHING, I think this should be fine. As noted in
> > the comments, this might be a better way of handling the static lambda
> > case too. This is still a nasty hack so it should probably be done
> > differently, but I question if making a whole new fntype node in
> > tsubst_lambda_expr makes sense. On the other hand, maybe there will be
> > problems if a lambda is already instantiated? I'm not sure, mutating
> > things this way makes me uneasy though.
>
>
> A problem with changing TYPE_METHOD_BASETYPE for a FUNCTION_TYPE is that
> all equivalent FUNCTION_TYPE share the same tree node (through
> type_hash_canon), so you're messing with the type of unrelated functions
> at the same time. I think it's better to stick with the way static
> lambdas are handled.
Yes, that was my concern as well, without even thinking about hashing.
I will investigate it deeper but I'm pretty sure TYPE_METHOD_BASETYPE,
while a valid field, is not used at all for function_type nodes. I had
to look into this when looking into the DECL_CONTEXT stuff previously,
so I'm pretty confident in this. Come to think of it, does hashing even
take fields that aren't "supposed" to be set for a node into account?
Well, even so, I can just clear it after it gets used as we just need
it to pass the closure type down. Perhaps I should have led with this,
but as it stands the version that uses TYPE_METHOD_BASETYPE bootstraps
with no regressions. I'll still look deeper but I'm pretty confident in
my decision here, I really don't want to try to unravel what
build_memfn_type does, I would rather find a whole different way of
passing that information down.
> > I don't like that pt.cc feels like it has a ton of hidden mutations,
> > it's really hard to follow through it. Would you agree it's in need for
> > cleanup or am I just not experienced enough in this area yet?
>
>
> I'm sure there are things that could use cleaning up, but I'm not
> thinking of anything specific offhand. Any particular examples?
Not at the moment, just after 8 hours of following the execution path
you start to feel like something could be done better. I'm sorry if I
sound bitter but to be honest I kind of am. :^)
I don't think it's going to be my first goal to look at pt.cc for
things to clean up, I honestly didn't want to go through here at all.
Maybe if I look at it for longer I will feel better about it but I want
to make that a task for the future.
> > Regarding the error handling, I just had a thought about it, I have a
> > hunch it definitely needs to go in tsubst_template_decl or
> > tsubst_function_decl. There might need to be more changes to determine
> > the actual type of the lambda in there, but everything else I've done
> > changes the implicit object argument to be treated more like a regular
> > argument, doing error handling for the object type in
> > tsubst_lambda_expr would be inconsistent with that.
>
>
> But the rule about unrelated type is specific to lambdas, so doing it in
> tsubst_lambda_expr seems preferable to adding a lambda special case to
> generic code.
No, while that might seem intuitive, the error is not during
instantiation of the closure object's type, it's during instantiation
of the function template. The type of the closure will always be the
type of the closure, but the type of the explicit object parameter can
be different. In the following example we don't go into
tsubst_lambda_expr at all.
template
struct S : T {
using T::operator();
using s_tag = void;
};
int main()
{
auto f = [](this auto self){
if constexpr ( requires{typename decltype(self)::s_tag;} ) {
return 5;
}
else {
return 10;
}
};
auto s = S{f};
if (f () != 10)
__builtin_abort ();
if (s () != 5)
__builtin_abort ();
}
I hope this demonstrates that placing the check in tsubst_function_decl
is the correct way to do this.
Now with that out of the way, I do still kind of feel icky about it.
This really is a MASSIVE edge case that will almost never matter. As
far as I know, instantiating explicitly like so...
auto f = [x = 42](this auto&&) -> int { return x; };
int (*fp)(int&) = (f)::operator();
...is the only way to coerce the explicit object parameter of the
lambda's call operator into deducing as an unrelated type. Cases that
are not deduced can be caught trivially while parsing the lambda and
are the only reasonable cases where you might have an unrelated type.
Perhaps it might become relevant in the future if a proposal like
https://atomgalaxy.github.io/isocpp-1107/D1107.html ever made it in,
but we aren't there yet. So as it stands, I'm pretty certain that it's
just impossible t
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
use problems to try to move away from that. I think I'll probably start to mess around with this idea as soon as this patch is done. That unrelated rambling aside, I will get back to work and try my best to finish the errors for unrelated types today. It feels kind of bad to work so hard on this since I've noticed that it's almost impossible to coerce the case other than overload resolution when taking the address of a lambdas operator. But that's not an excuse to leave a hole in the compiler so, it's still gotta get done. I don't think I will get rejection of mutation of captures done, especially since I'm uncertain of what the right way to do it is, but maybe I'll work fast. On Saturday, November 25th, 2023 at 10:32 AM, Jason Merrill wrote: > > > On 11/24/23 01:49, waffl3x wrote: > > > > and this in tsubst_lambda_expr that assumes iobj: > > > > > > /* Fix the type of 'this'. */ > > > fntype = build_memfn_type (fntype, type, > > > type_memfn_quals (fntype), > > > type_memfn_rqual (fntype)); > > > > Unfortunately, putting a condition on this had some unforeseen > > consequences. I've been working on this about 8 hours today and I'm a > > little defeated after discovering this. > > > > commit 39ade88fa1632c659c5c4ed065fa2b62d16a8670 > > Author: Jason Merrill ja...@redhat.com > > Date: Tue Jan 24 15:29:35 2023 -0500 > > > > c++: static lambda in template [PR108526] > > > > tsubst_lambda_expr uses build_memfn_type to build a METHOD_TYPE for the new > > lamba op(). This is not what we want for a C++23 static op(), but since we > > also use that METHOD_TYPE to communicate the closure type down to > > tsubst_function_decl, let's wait and turn it back at that point. > > > > PR c++/108526 > > > > gcc/cp/ChangeLog: > > > > gcc/cp/ChangeLog: > > > > * pt.cc (tsubst_function_decl): Handle static lambda. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp23/static-operator-call5.C: New test. > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index 2a4d03c5e47..51fc246ed71 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -14306,6 +14306,11 @@ tsubst_function_decl (tree t, tree args, > > tsubst_flags_t complain, > > tree ctx = closure ? closure : DECL_CONTEXT (t); > > bool member = ctx && TYPE_P (ctx); > > > > + /* If this is a static lambda, remove the 'this' pointer added in > > + tsubst_lambda_expr now that we know the closure type. */ > > + if (lambda_fntype && DECL_STATIC_FUNCTION_P (t)) > > + lambda_fntype = static_fn_type (lambda_fntype); > > > Ah, right. So the simplest thing would be to do the same thing here for > xob lambdas. > > Jason
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
OKAY, I figured out SOMETHING, I think this should be fine. As noted in
the comments, this might be a better way of handling the static lambda
case too. This is still a nasty hack so it should probably be done
differently, but I question if making a whole new fntype node in
tsubst_lambda_expr makes sense. On the other hand, maybe there will be
problems if a lambda is already instantiated? I'm not sure, mutating
things this way makes me uneasy though.
I don't like that pt.cc feels like it has a ton of hidden mutations,
it's really hard to follow through it. Would you agree it's in need for
cleanup or am I just not experienced enough in this area yet?
I still have to write good tests for this case and move the error
handling for unrelated object types into here, but my gut tells me this
issue should be fixed now.
Regarding the error handling, I just had a thought about it, I have a
hunch it definitely needs to go in tsubst_template_decl or
tsubst_function_decl. There might need to be more changes to determine
the actual type of the lambda in there, but everything else I've done
changes the implicit object argument to be treated more like a regular
argument, doing error handling for the object type in
tsubst_lambda_expr would be inconsistent with that.
Also, before I forget, you were totally right about just using
dependent_type_p in cp_parser_lambda_declarator_opt. It really is just
that simple.
The code below is not necessarily final, but I wanted to get your
thoughts about doing it this way given my concerns written above. Now
that this problem is out of the way, or at least almost out of the way,
I hopefully can get a version pushed out today.
Alex
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 86c95b278ba..ea2db5c0c9d 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -14407,14 +14407,20 @@ tsubst_function_decl (tree t, tree args,
tsubst_flags_t complain,
gen_tmpl = NULL_TREE;
argvec = NULL_TREE;
}
-
+ /* We hack TYPE_METHOD_BASETYPE onto xobj member functions in
+ tsubst_lambda_expr to get the proper closure type here. */
tree closure = (lambda_fntype ? TYPE_METHOD_BASETYPE (lambda_fntype)
: NULL_TREE);
tree ctx = closure ? closure : DECL_CONTEXT (t);
bool member = ctx && TYPE_P (ctx);
/* If this is a static lambda, remove the 'this' pointer added in
- tsubst_lambda_expr now that we know the closure type. */
+ tsubst_lambda_expr now that we know the closure type.
+ I suspect that we can just carry this information down in
+ TYPE_METHOD_BASETYPE without building the full method type in
+ tsubst_lambda_expr. This wouldn't be ideal but neither is this.
+ Since that field isn't used for anything for static or xobj member
+ functions, it should be fine to do that. */
if (lambda_fntype && DECL_STATIC_FUNCTION_P (t))
lambda_fntype = static_fn_type (lambda_fntype);
@@ -14490,12 +14496,12 @@ tsubst_function_decl (tree t, tree args,
tsubst_flags_t complain,
DECL_NAME (r) = make_conv_op_name (TREE_TYPE (type));
tree parms = DECL_ARGUMENTS (t);
- if (closure && !DECL_STATIC_FUNCTION_P (t))
+ if (closure && DECL_IOBJ_MEMBER_FUNC_P (t))
parms = DECL_CHAIN (parms);
parms = tsubst (parms, args, complain, t);
for (tree parm = parms; parm; parm = DECL_CHAIN (parm))
DECL_CONTEXT (parm) = r;
- if (closure && !DECL_STATIC_FUNCTION_P (t))
+ if (closure && DECL_IOBJ_MEMBER_FUNC_P (t))
{
tree tparm = build_this_parm (r, closure, type_memfn_quals (type));
DECL_NAME (tparm) = closure_identifier;
@@ -19491,9 +19497,16 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
cp_evaluated ev;
/* Fix the type of 'this'. */
- fntype = build_memfn_type (fntype, type,
-type_memfn_quals (fntype),
-type_memfn_rqual (fntype));
+ if (DECL_IOBJ_MEMBER_FUNC_P (oldfn))
+ fntype = build_memfn_type (fntype, type,
+ type_memfn_quals (fntype),
+ type_memfn_rqual (fntype));
+ /* We don't use this field anywhere else for xobj member functions, and
+we need a way to pass this information down. Theres some really
+convoluted stuff going on unfortunately, and I can only begin to
+unravel it all. */
+ if (DECL_XOBJ_MEMBER_FUNC_P (oldfn))
+ TYPE_METHOD_BASETYPE (fntype) = type;
tree inst = (oldtmpl
? tsubst_template_decl (oldtmpl, args, complain,
fntype, tparms)
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Okay within about half an hour today I think I've figured out what the main issue is for my problem. For the static lambda case, while I don't like the hack that's currently present, I reckon we just leave it as it is, as it works just fine right now. For my issue, I'm not totally sure how to tackle it, but here's what I've identified. build_memfn_type calls into build_method_type_directly and that takes the main variant of the passed in basetype and sets it as the TYPE_METHOD_BASETYPE. TYPE_METHOD_BASETYPE (t) = TYPE_MAIN_VARIANT (basetype); So naively just bypassing build_memfn_type is not going to work, I'm not totally sure how to solve this though. (Below) /* Fix the type of 'this'. */ fntype = build_memfn_type (fntype, type, type_memfn_quals (fntype), type_memfn_rqual (fntype)); Making tree nodes in general is something I'm not very familiar with, and struggled with every time I tried to read any of the code. So for now I'm going to hope that the next block (below) handles everything properly as far as creating a new node and substitution goes. tree inst = (oldtmpl ? tsubst_template_decl (oldtmpl, args, complain, fntype, tparms) : tsubst_function_decl (oldfn, args, complain, fntype)); And I'll just see if setting the decl_context of inst to type does the trick. I guess I forgot to elaborate that tsubst_function_decl uses the decl_context of t on the second, which ends up having template-info attached to it. As you might have been able to deduce from what I've said so far, I suspect that it's because I'm not setting it properly when the function_decl is substituted into. I don't really know where the right place to do that is though, or really why it's not happening already, so I'm just gonna poke around, try some things, and clean it up later if it's messy. That's my quick update, hopefully this gets me somewhere finally. Alex On Thursday, November 23rd, 2023 at 11:49 PM, waffl3x wrote: > > > > and this in tsubst_lambda_expr that assumes iobj: > > > /* Fix the type of 'this'. */ > > fntype = build_memfn_type (fntype, type, > > type_memfn_quals (fntype), > > type_memfn_rqual (fntype)); > > > Unfortunately, putting a condition on this had some unforeseen > consequences. I've been working on this about 8 hours today and I'm a > little defeated after discovering this. > > commit 39ade88fa1632c659c5c4ed065fa2b62d16a8670 > Author: Jason Merrill ja...@redhat.com > > Date: Tue Jan 24 15:29:35 2023 -0500 > > c++: static lambda in template [PR108526] > > tsubst_lambda_expr uses build_memfn_type to build a METHOD_TYPE for the new > lamba op(). This is not what we want for a C++23 static op(), but since we > also use that METHOD_TYPE to communicate the closure type down to > tsubst_function_decl, let's wait and turn it back at that point. > > PR c++/108526 > > gcc/cp/ChangeLog: > > gcc/cp/ChangeLog: > > * pt.cc (tsubst_function_decl): Handle static lambda. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp23/static-operator-call5.C: New test. > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 2a4d03c5e47..51fc246ed71 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -14306,6 +14306,11 @@ tsubst_function_decl (tree t, tree args, > tsubst_flags_t complain, > tree ctx = closure ? closure : DECL_CONTEXT (t); > bool member = ctx && TYPE_P (ctx); > > + /* If this is a static lambda, remove the 'this' pointer added in > + tsubst_lambda_expr now that we know the closure type. */ > + if (lambda_fntype && DECL_STATIC_FUNCTION_P (t)) > + lambda_fntype = static_fn_type (lambda_fntype); > + > > I discovered this when I decided to try a static lambda to see if that > would help me narrow down my current problem. I was shocked to find out > it exhibited the exact same ICE I was trying to fix. So I was going to > go undo my changes one by one to see what it was, thankfully this was > the first one I tried, I undid the condition I put on it, and the crash > was gone. > > Anyway, this has been my whole day so far, I am going to have to look > deeper to decide how exactly I fix this because I don't think this hack > that is in place at the moment is the right way to do it. The first > idea that comes to mind is modifying the decl_context of the call > operator, but I'm really not sure. I'm just going to take a break, eat > some pizza, and come back to this hopefully less defeated. > > Alex
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> and this in tsubst_lambda_expr that assumes iobj: > > /* Fix the type of 'this'. */ > fntype = build_memfn_type (fntype, type, > type_memfn_quals (fntype), > type_memfn_rqual (fntype)); Unfortunately, putting a condition on this had some unforeseen consequences. I've been working on this about 8 hours today and I'm a little defeated after discovering this. commit 39ade88fa1632c659c5c4ed065fa2b62d16a8670 Author: Jason Merrill Date: Tue Jan 24 15:29:35 2023 -0500 c++: static lambda in template [PR108526] tsubst_lambda_expr uses build_memfn_type to build a METHOD_TYPE for the new lamba op(). This is not what we want for a C++23 static op(), but since we also use that METHOD_TYPE to communicate the closure type down to tsubst_function_decl, let's wait and turn it back at that point. PR c++/108526 gcc/cp/ChangeLog: gcc/cp/ChangeLog: * pt.cc (tsubst_function_decl): Handle static lambda. gcc/testsuite/ChangeLog: * g++.dg/cpp23/static-operator-call5.C: New test. diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 2a4d03c5e47..51fc246ed71 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -14306,6 +14306,11 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, tree ctx = closure ? closure : DECL_CONTEXT (t); bool member = ctx && TYPE_P (ctx); + /* If this is a static lambda, remove the 'this' pointer added in + tsubst_lambda_expr now that we know the closure type. */ + if (lambda_fntype && DECL_STATIC_FUNCTION_P (t)) +lambda_fntype = static_fn_type (lambda_fntype); + I discovered this when I decided to try a static lambda to see if that would help me narrow down my current problem. I was shocked to find out it exhibited the exact same ICE I was trying to fix. So I was going to go undo my changes one by one to see what it was, thankfully this was the first one I tried, I undid the condition I put on it, and the crash was gone. Anyway, this has been my whole day so far, I am going to have to look deeper to decide how exactly I fix this because I don't think this hack that is in place at the moment is the right way to do it. The first idea that comes to mind is modifying the decl_context of the call operator, but I'm really not sure. I'm just going to take a break, eat some pizza, and come back to this hopefully less defeated. Alex
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Wednesday, November 22nd, 2023 at 2:38 PM, Jason Merrill wrote: > > > On 11/22/23 15:46, waffl3x wrote: > > > On Tuesday, November 21st, 2023 at 8:22 PM, Jason Merrill ja...@redhat.com > > wrote: > > > > > On 11/21/23 08:04, waffl3x wrote: > > > > > > > /* Nonzero for FUNCTION_DECL means that this decl is a non-static > > > > - member function. */ > > > > + member function, use DECL_IOBJ_MEMBER_FUNC_P instead. */ > > > > #define DECL_NONSTATIC_MEMBER_FUNCTION_P(NODE) \ > > > > (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE) > > > > > > > > +/* Nonzero for FUNCTION_DECL means that this decl is an implicit object > > > > + member function. */ > > > > +#define DECL_IOBJ_MEMBER_FUNC_P(NODE) \ > > > > + (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE) > > > > > > I was thinking to rename DECL_NONSTATIC_MEMBER_FUNCTION_P rather than > > > add a new, equivalent one. And then go through all the current uses of > > > the old macro to decide whether they mean IOBJ or OBJECT. > > > > I figure it would be easier to make that transition if there's a clear > > line between old versus new. To be clear, my intention is for the old > > macro to be removed once all the uses of it are changed over to the new > > macro. I can still remove it for the patch if you like but having both > > and removing the old one later seems better to me. > > > Hmm, I think changing all the uses is a necessary part of this change. > I suppose it could happen before the main patch, if you'd prefer, but it > seems more straightforward to include it. > > > > > + else if (declarator->declarator->kind == cdk_pointer) > > > > + error_at (DECL_SOURCE_LOCATION (xobj_parm), > > > > + /* "a pointer to function type cannot "? */ > > > > + "a function pointer type cannot " > > > > + "have an explicit object parameter"); > > > > > > "pointer to function type", yes. > > > > > > > + /* The locations being used here are probably not correct. */ > > > > > > Why not? > > > > I threw them in just so I could call inform, but it doesn't feel like > > the messages should be pointing at the parameter, but rather at the > > full type declaration. When I put those locations in I wasn't sure how > > to get the full declaration location, and I'm still not 100% confident > > in how to do it, so I just threw them in and moved on. > > > That would be more precise, but I think it's actually preferable for the > inform to have the same location as the previous error to avoid > redundant quoting of the source. Yeah that makes sense, I'll revise the comment with that rationale and we can maybe revisit it later. > > > Let's clear xobj_parm after giving an error in the TYPENAME case > > > > I don't like the spirit of this very much, whats your reasoning for > > this? We're nearly at the end of the scope where it is last used, I > > think it would be more unclear if we suddenly set it to NULL_TREE near > > the end. It raises the question of whether that assignment actually > > does anything, or if we are just trying to indicate that it isn't being > > used anymore, but I already made sure to declare it in the deepest > > scope possible. That much should be sufficient for indicating it's > > usage, no? > > > Hmm, I think I poked at that and changed my mind, but forgot to delete > the suggestion. Never mind. Perfect. > > > > if ((!methodp && !DECL_XOBJ_MEMBER_FUNC_P (decl)) > > > > || DECL_STATIC_FUNCTION_P (decl)) > > > > > > I think this can just be if (DECL_OBJECT_MEMBER_FUNC_P (decl)). > > > > Alright, and going forward I'll try to make more changes that are > > consistent with this one. With that said I'm not sure it can, but I'll > > take a close look and if you're right I'll make that change. > > > > > > if (TREE_CODE (fntype) == METHOD_TYPE) > > > > ctype = TYPE_METHOD_BASETYPE (fntype); > > > > + else if (DECL_XOBJ_MEMBER_FUNC_P (decl1)) > > > > + ctype = DECL_CONTEXT (decl1); > > > > > > All of this can be > > > > > > if (DECL_CLASS_SCOPE_P (decl1)) > > > ctype = DECL_CONTEXT (decl1); > > > > > > I think I'm going to go ahead and clean that up now. > > > > Sounds good to me, a lot of this stuff needs small cleanups and I'm > > just concerned about making them too much. > > > My cl
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Tuesday, November 21st, 2023 at 8:22 PM, Jason Merrill wrote: > > > On 11/21/23 08:04, waffl3x wrote: > > > Bootstrapped and tested on x86_64-linux with no regressions. > > > > Hopefully this patch is legible enough for reviewing purposes, I've not > > been feeling the greatest so it was a task to get this finished. > > Tomorrow I will look at putting the diagnostics in > > start_preparsed_function and also fixing up anything else. > > > > To reiterate in case it wasn't abundantly clear by the barren changelog > > and commit message, this version is not intended as the final revision. > > > > Handling re-declarations was kind of nightmarish, so the comments in > > there are lengthy, but I am fairly certain I implemented them correctly. > > > > I am going to get some sleep now, hopefully I will feel better tomorrow > > and be ready to polish off the patch. Thanks for the patience. > > > Great! > > > I stared at start_preparsed_function for a long while and couldn't > > figure out where to start off at. So for now the error handling is > > split up between instantiate_body and cp_parser_lambda_declarator_opt. > > The latter is super not correct but I've been stuck on this for a long > > time now though so I wanted to actually get something that works and > > then try to make it better. > > > I see what you mean, instantiate body isn't prepared for > start_preparsed_function to fail. It's ok to handle this in two places. > Though actually, instantiate_body is too late for it to fail; I think > for the template case it should fail in tsubst_lambda_expr, before we > even start to consider the body. > > Incidentally, I notice this code in tsubst_function_decl seems to need > adjusting for xobj: > > tree parms = DECL_ARGUMENTS (t); > if (closure && !DECL_STATIC_FUNCTION_P (t)) > parms = DECL_CHAIN (parms); > parms = tsubst (parms, args, complain, t); > for (tree parm = parms; parm; parm = DECL_CHAIN (parm)) > DECL_CONTEXT (parm) = r; > if (closure && !DECL_STATIC_FUNCTION_P (t)) > ... > > and this in tsubst_lambda_expr that assumes iobj: > > /* Fix the type of 'this'. */ > fntype = build_memfn_type (fntype, type, > type_memfn_quals (fntype), > type_memfn_rqual (fntype)); Originally I was going to say this doesn't look like a problem in tsubst_lambda_expr, but after looking at tsubst_function_decl I'm thinking it might be the source of some trouble. If it really was causing problems I would think it would be working much worse than it currently is, but it does feel like it might be the actual source of the bug I was chasing yesterday. Assigning to a capture with a deduced const xobj parameter is not being rejected right now, this might be causing it. I'll look more thoroughly today. > This also seems like the place to check for unrelated type. It does feel that way, I agree. > > /* Nonzero for FUNCTION_DECL means that this decl is a non-static > > - member function. */ > > + member function, use DECL_IOBJ_MEMBER_FUNC_P instead. */ > > #define DECL_NONSTATIC_MEMBER_FUNCTION_P(NODE) \ > > (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE) > > > > +/* Nonzero for FUNCTION_DECL means that this decl is an implicit object > > + member function. */ > > +#define DECL_IOBJ_MEMBER_FUNC_P(NODE) \ > > + (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE) > > > I was thinking to rename DECL_NONSTATIC_MEMBER_FUNCTION_P rather than > add a new, equivalent one. And then go through all the current uses of > the old macro to decide whether they mean IOBJ or OBJECT. I figure it would be easier to make that transition if there's a clear line between old versus new. To be clear, my intention is for the old macro to be removed once all the uses of it are changed over to the new macro. I can still remove it for the patch if you like but having both and removing the old one later seems better to me. > > - (static or non-static). */ > > + (static or object). */ > > > Let's leave this comment as it was. Okay. > > + auto handle_arg = [fn, flags, complain](tree type, > > + tree arg, > > + int const param_index, > > + conversion *conv, > > + bool const conversion_warning) > > > Let's move the conversion_warning logic into the handle_arg lambda > rather than have it as a parameter. Yes, we don't need it for the xobj > parm, but I think it's cleaner to have less in the loop. I would argue that it's cleaner to have the lambda be concise, but I'll make this change. > Also, let's move handle_arg after the iobj 'this' handling so it's > closer to the uses. For which the 'else xobj' needs to drop the 'else', > or change to 'if (
Re: [PATCH v5 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> > > > /* Nonzero for FUNCTION_DECL means that this decl is a non-static > > > > - member function. */ > > > > + member function, use DECL_IOBJ_MEMBER_FUNC_P instead. */ > > > > #define DECL_NONSTATIC_MEMBER_FUNCTION_P(NODE) \ > > > > (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE) > > > > > > > > +/* Nonzero for FUNCTION_DECL means that this decl is an implicit object > > > > + member function. */ > > > > +#define DECL_IOBJ_MEMBER_FUNC_P(NODE) \ > > > > + (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE) > > > > > > I was thinking to rename DECL_NONSTATIC_MEMBER_FUNCTION_P rather than > > > add a new, equivalent one. And then go through all the current uses of > > > the old macro to decide whether they mean IOBJ or OBJECT. > > > > I figure it would be easier to make that transition if there's a clear > > line between old versus new. To be clear, my intention is for the old > > macro to be removed once all the uses of it are changed over to the new > > macro. I can still remove it for the patch if you like but having both > > and removing the old one later seems better to me. > > > Hmm, I think changing all the uses is a necessary part of this change. > I suppose it could happen before the main patch, if you'd prefer, but it > seems more straightforward to include it. > I had meant to reply to this as well but forgot, I agree that it's likely necessary but I've only been changing them as I come across things that don't work right rather than trying to evaluate them through the code. Making changes to them without having a test case that demonstrates that the case is definitely being handled incorrectly is risky, especially for me since I don't have a full understanding of the code base. I would rather only change ones that are evidently wrong, and defer the rest to someone else who knows the code base better. With that said, I have been neglecting replacing uses of the old macro, but I now realize that's just creating more work for whoever is evaluating the rest of them. Going forward I will make sure I replace the old macro when I am fairly certain it should be. Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Monday, November 20th, 2023 at 7:35 AM, Jason Merrill
wrote:
>
>
> On 11/19/23 16:44, waffl3x wrote:
>
> > On Sunday, November 19th, 2023 at 1:34 PM, Jason Merrill ja...@redhat.com
> > wrote:
> >
> > > On 11/19/23 13:36, waffl3x wrote:
> > >
> > > > I'm having trouble fixing the error for this case, the control flow
> > > > when the functions are overloaded is much more complex.
> > > >
> > > > struct S {
> > > > void f(this S&) {}
> > > > void f(this S&, int)
> > > >
> > > > void g() {
> > > > void (*fp)(S&) =
> > > > }
> > > > };
> > > >
> > > > This seemed to have fixed the non overloaded case, but I'm also not
> > > > very happy with it, it feels kind of icky. Especially since the expr's
> > > > location isn't available here, although, it just occurred to me that
> > > > the expr's location is probably stored in the node.
> > > >
> > > > typeck.cc:cp_build_addr_expr_1
> > > > ```
> > > > case BASELINK:
> > > > arg = BASELINK_FUNCTIONS (arg);
> > > > if (DECL_XOBJ_MEMBER_FUNC_P (
> > > > {
> > > > error ("You must qualify taking address of xobj member functions");
> > > > return error_mark_node;
> > > > }
> > >
> > > The loc variable was set earlier in the function, you can use that.
> >
> > Will do.
> >
> > > The overloaded case we want to handle here in
> > > resolve_address_of_overloaded_function:
> > >
> > > > if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
> > > > && !(complain & tf_ptrmem_ok) && !flag_ms_extensions)
> > > > {
> > > > static int explained;
> > > >
> > > > if (!(complain & tf_error))
> > > > return error_mark_node;
> > > >
> > > > auto_diagnostic_group d;
> > > > if (permerror (input_location, "assuming pointer to member %qD", fn)
> > > > && !explained)
> > > > {
> > > > inform (input_location, "(a pointer to member can only be "
> > > > "formed with %<&%E%>)", fn);
> > > > explained = 1;
> > > > }
> > > > }
> > >
> > > Jason
> >
> > I'll check that out now, I just mostly finished the first lambda crash.
> >
> > What is the proper way to error out of instantiate_body? What I have
> > right now is just not recursing down further if theres a problem. Also,
> > I'm starting to wonder if I should actually be erroring in
> > instantiate_decl instead.
>
>
> I think you want to error in start_preparsed_function, to handle
> template and non-template cases in the same place.
>
> Jason
I just started a bootstrap, hopefully everything comes out just fine.
If I don't pass out before the tests finish (and the tests are all
fine) then I'll send it in for review tonight.
I stared at start_preparsed_function for a long while and couldn't
figure out where to start off at. So for now the error handling is
split up between instantiate_body and cp_parser_lambda_declarator_opt.
The latter is super not correct but I've been stuck on this for a long
time now though so I wanted to actually get something that works and
then try to make it better.
This patch is not as final as I would have liked as you can probably
deduce from the previous paragraph. I still have to write tests for a
number of cases, but I'm pretty sure everything works. I was going to
say except for by-value xobj parameters in lambdas with captures, but
that's magically working now too. I was also going to say I don't know
why, but I found where my mistake was (that I fixed without realizing
it was causing the aforementioned problem) so I do know what did it.
So rambling aside, I think everything should work now. To reiterate, I
still have to finish the tests for a few things. There's some
diagnostics I'm super not happy with, and lambda's names still say
static, but I already know how to fix that I think.
I will make an attempt at moving the diagnostics for an unrelated
explicit object parameter in a lambda with captures tomorrow. I just
want to get the almost fully featured patch reviewed ASAP, even if it's
still got some cruft.
As soon as these tests pass I will submit the patch, I'm not going to
split it up today, I'm simply too tired, but I assure you the final
version will properly be split up with a correct commit message and
everything.
Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Sunday, November 19th, 2023 at 1:34 PM, Jason Merrill
wrote:
>
>
> On 11/19/23 13:36, waffl3x wrote:
>
> > I'm having trouble fixing the error for this case, the control flow
> > when the functions are overloaded is much more complex.
> >
> > struct S {
> > void f(this S&) {}
> > void f(this S&, int)
> >
> > void g() {
> > void (*fp)(S&) =
> > }
> > };
> >
> > This seemed to have fixed the non overloaded case, but I'm also not
> > very happy with it, it feels kind of icky. Especially since the expr's
> > location isn't available here, although, it just occurred to me that
> > the expr's location is probably stored in the node.
> >
> > typeck.cc:cp_build_addr_expr_1
> > ```
> > case BASELINK:
> > arg = BASELINK_FUNCTIONS (arg);
> > if (DECL_XOBJ_MEMBER_FUNC_P (
> > {
> > error ("You must qualify taking address of xobj member functions");
> > return error_mark_node;
> > }
>
>
> The loc variable was set earlier in the function, you can use that.
Will do.
> The overloaded case we want to handle here in
> resolve_address_of_overloaded_function:
>
> > if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
> > && !(complain & tf_ptrmem_ok) && !flag_ms_extensions)
> > {
> > static int explained;
> >
> > if (!(complain & tf_error))
> > return error_mark_node;
> >
> > auto_diagnostic_group d;
> > if (permerror (input_location, "assuming pointer to member %qD", fn)
> > && !explained)
> > {
> > inform (input_location, "(a pointer to member can only be "
> > "formed with %<&%E%>)", fn);
> > explained = 1;
> > }
> > }
>
>
> Jason
I'll check that out now, I just mostly finished the first lambda crash.
What is the proper way to error out of instantiate_body? What I have
right now is just not recursing down further if theres a problem. Also,
I'm starting to wonder if I should actually be erroring in
instantiate_decl instead.
I guess it will be better to just finish and you can share your
comments upon review though.
Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Sunday, November 19th, 2023 at 9:31 AM, Jason Merrill
wrote:
>
>
> On 11/19/23 08:39, waffl3x wrote:
>
> > Funny enough I ended up removing the ones I was thinking about, seems
> > to always happen when I ask style questions but I'm glad to hear it's
> > okay going forward.
> >
> > I'm having trouble fixing this bug, based on what Gasper said in
> > PR102609 I am pretty sure I know what the semantics should be. Since
> > the capture is not used in the body of the function, it should be well
> > formed to call the function with an unrelated type.
>
>
> I don't think so: https://eel.is/c++draft/expr.prim#lambda.closure-5
> says the type of the xobj parameter must be related.
Well, thanks for bringing that to my attention, that makes things
easier but I'm kinda disappointed, I almost wanted an excuse to write
more code. I wonder why Gasper thought this wasn't the case, perhaps it
was a later decision?
Okay, I checked, that paragraph is in the original paper, I thought I
could just take what one of the paper's authors said for granted but I
guess not, I'm not going to make that mistake again. On the other hand,
maybe he just misunderstood my question, what can you do.
Well regardless, that reduces the things I have left to do by a whole
lot.
> > I had begun trying to tackle the case that Gasper mentioned and got the
> > following ICE. I also have another case that ICEs so I've been thinking
> > I don't get to do little changes to fix this. I've been looking at this
> > for a few hours now and given we are past the deadline now I figured I
> > should see what others think.
> >
> > int main()
> > {
> > int x = 42;
> > auto f1 = [x](this auto&& self) {};
> >
> > static_cast(decltype(f1)::operator());
> > }
>
>
> This should be rejected when we try to instantiate the op() with int.
Yep, absolutely, that is clear now.
Just for the record, clang accepts it, but since I can't think of any
use cases for this I don't think there's any value in supporting it.
>
> > As I said, there is also this case that also ICEs in the same region.
> > It's making me think that some core assumptions are being violated in
> > the code leading up to finish_non_static_data_member.
> >
> > int main()
> > {
> > int x = 42;
> > auto f1 = [x](this auto self) {};
> > }
>
>
> Here I think the problem is in build_capture_proxy:
>
> > /* The proxy variable forwards to the capture field. */
> > object = build_fold_indirect_ref (DECL_ARGUMENTS (fn));
> > object = finish_non_static_data_member (member, object, NULL_TREE);
>
>
> The call to build_fold_indirect_ref assumes that 'this' is a pointer,
> which it is not here. I think you can just make that conditional on it
> being a pointer or reference?
>
> Jason
Thanks, I will take a look at that area.
I'm having trouble fixing the error for this case, the control flow
when the functions are overloaded is much more complex.
struct S {
void f(this S&) {}
void f(this S&, int)
void g() {
void (*fp)(S&) =
}
};
This seemed to have fixed the non overloaded case, but I'm also not
very happy with it, it feels kind of icky. Especially since the expr's
location isn't available here, although, it just occurred to me that
the expr's location is probably stored in the node.
typeck.cc:cp_build_addr_expr_1
```
case BASELINK:
arg = BASELINK_FUNCTIONS (arg);
if (DECL_XOBJ_MEMBER_FUNC_P (
{
error ("You must qualify taking address of xobj member functions");
return error_mark_node;
}
Anyway, I'm quite tired but I'll to finish off the lambda stuff before
calling it, then I'll run a bootstrap and tests and if all is well I
will submit the patch. I will probably skimp on the changelog and
commit message as that's the part I have the hardest time on,
especially when I'm tired.
Alex
```
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Funny enough I ended up removing the ones I was thinking about, seems
to always happen when I ask style questions but I'm glad to hear it's
okay going forward.
I'm having trouble fixing this bug, based on what Gasper said in
PR102609 I am pretty sure I know what the semantics should be. Since
the capture is not used in the body of the function, it should be well
formed to call the function with an unrelated type.
I had begun trying to tackle the case that Gasper mentioned and got the
following ICE. I also have another case that ICEs so I've been thinking
I don't get to do little changes to fix this. I've been looking at this
for a few hours now and given we are past the deadline now I figured I
should see what others think.
int main()
{
int x = 42;
auto f1 = [x](this auto&& self) {};
static_cast(decltype(f1)::operator());
}
explicit-obj-lambdaX3.C: In instantiation of 'main():: static
[with auto:1 = int&]':
explicit-obj-lambdaX3.C:33:53: required from here
33 | static_cast(decltype(f1)::operator());
| ^
explicit-obj-lambdaX3.C:31:33: internal compiler error: tree check: expected
record_type or union_type or qual_union_type, have integer_type in
finish_non_static_data_member, at cp/semantics.cc:2294
31 | auto f1 = [x](this auto&& self) {};
| ^
0x1c66dda tree_check_failed(tree_node const*, char const*, int, char const*,
...)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/tree.cc:8949
0xb2e125 tree_check3(tree_node*, char const*, int, char const*, tree_code,
tree_code, tree_code)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/tree.h:3638
0xedfaf4 finish_non_static_data_member(tree_node*, tree_node*, tree_node*, int)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/semantics.cc:2294
0xe8b9b8 tsubst_expr(tree_node*, tree_node*, int, tree_node*)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/pt.cc:20864
0xe6d713 tsubst_decl
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/pt.cc:15387
0xe6fb1b tsubst(tree_node*, tree_node*, int, tree_node*)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/pt.cc:15967
0xe7bd81 tsubst_stmt
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/pt.cc:18299
0xe7df18 tsubst_stmt
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/pt.cc:18554
0xea6982 instantiate_body
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/pt.cc:26743
0xea83e9 instantiate_decl(tree_node*, bool, bool)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/pt.cc:27030
0xb5f9c9 resolve_address_of_overloaded_function
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/class.cc:8802
0xb60be1 instantiate_type(tree_node*, tree_node*, int)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/class.cc:9061
0xaf9992 standard_conversion
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/call.cc:1244
0xafcb57 implicit_conversion
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/call.cc:2081
0xb2a8cb perform_direct_initialization_if_possible(tree_node*, tree_node*,
bool, int)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/call.cc:13456
0xf69db8 build_static_cast_1
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/typeck.cc:8356
0xf6af1b build_static_cast(unsigned int, tree_node*, tree_node*, int)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/typeck.cc:8566
0xd9fc02 cp_parser_postfix_expression
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/parser.cc:7531
0xda45af cp_parser_unary_expression
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/parser.cc:9244
0xda5db4 cp_parser_cast_expression
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/parser.cc:10148
As I said, there is also this case that also ICEs in the same region.
It's making me think that some core assumptions are being violated in
the code leading up to finish_non_static_data_member.
int main()
{
int x = 42;
auto f1 = [x](this auto self) {};
}
explicit-obj-lambdaX3.C: In lambda function:
explicit-obj-lambdaX3.C:31:31: internal compiler error: Segmentation fault
31 | auto f1 = [x](this auto self) {};
| ^
0x1869eaa crash_signal
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/toplev.cc:315
0xb2ea0b strip_array_types(tree_node*)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/tree.h:4955
0xf773e2 cp_type_quals(tree_node const*)
/home/waffl3x/projects/gcc-dev/workspace/src/xobj-next/gcc/cp/typeck.cc:11509
0xedf993 finish_non_static_data_member(tree_node*, tree_node*, tree_node*, int)
/home/waffl3x/projects/gcc-de
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
The patch is coming along, I just have a quick question regarding
style. I make use of IILE's (immediately invoked lambda expression) a
whole lot in my own code. I know that their use is controversial in
general so I would prefer to ask instead of just submitting the patch
using them a bunch suddenly. I wouldn't have bothered either but this
part is really miserable without them.
If that would be okay, I would suggest an additional exception to
bracing style for lambdas.
This:
[](){
// stuff
};
Instead of this:
[]()
{
// stuff
};
This is especially important for IILE pattern IMO, else it looks really
mediocre. If this isn't okay okay I'll refactor all the IILE's that I
added, or just name them and call them instead. Whatever you think is
most appropriate.
Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
On Monday, November 13th, 2023 at 8:48 PM, Jason Merrill
wrote:
>
>
> On 11/11/23 05:43, waffl3x wrote:
>
> > > [combined reply to all three threads]
> > >
> > > On 11/9/23 23:24, waffl3x wrote:
> > >
> > > > > > There are a few known issues still present in this patch. Most
> > > > > > importantly,
> > > > > > the implicit object argument fails to convert when passed to
> > > > > > by-value xobj
> > > > > > parameters. This occurs both for xobj parameters that match the
> > > > > > argument type
> > > > > > and xobj parameters that are unrelated to the object type, but have
> > > > > > valid
> > > > > > conversions available. This behavior can be observed in the
> > > > > > explicit-obj-by-value[1-3].C tests. The implicit object argument
> > > > > > appears to be
> > > > > > simply reinterpreted instead of any conversion applied. This is
> > > > > > elaborated on
> > > > > > in the test cases.
> > > > >
> > > > > Yes, that's because of:
> > > > >
> > > > > > @@ -9949,7 +9951,8 @@ build_over_call (struct z_candidate cand, int
> > > > > > flags, tsubst_flags_t complain)
> > > > > > }
> > > > > > }
> > > > > > / Bypass access control for 'this' parameter. */
> > > > > > - else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE)
> > > > > > + else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
> > > > > > + || DECL_XOBJ_MEMBER_FUNC_P (fn))
> > > > >
> > > > > We don't want to take this path for xob fns. Instead I think we need
> > > > > to
> > > > > change the existing:
> > > > >
> > > > > > gcc_assert (first_arg == NULL_TREE);
> > > > >
> > > > > to assert that if first_arg is non-null, we're dealing with an xob fn,
> > > > > and then go ahead and do the same conversion as the loop body on
> > > > > first_arg.
> > > > >
> > > > > > Despite this, calls where there is no valid conversion
> > > > > > available are correctly rejected, which I find surprising. The
> > > > > > explicit-obj-by-value4.C testcase demonstrates this odd but correct
> > > > > > behavior.
> > > > >
> > > > > Yes, because checking for conversions is handled elsewhere.
> > > >
> > > > Yeah, as I noted above I realized that just handling it the same way as
> > > > iobj member functions is fundamentally broken. I was staring at it last
> > > > night and eventually realized that I could just copy the loop body. I
> > > > ended up asserting in the body handling the implicit object argument
> > > > for xobj member functions that first_arg != NULL_TREE, which I wasn't
> > > > sure of, but it seems to work.
> > >
> > > That sounds like it might cause trouble with
> > >
> > > struct A {
> > > void f(this A);
> > > };
> > >
> > > int main()
> > > {
> > > (::f) (A());
> > > }
> >
> > I will check to see what the behavior with this is. This sounds related
> > to the next question I asked as well.
> >
> > > > I tried asking in IRC if there are any circumstances where first_arg
> > > > would be null for a non-static member function and I didn't get an
> > > > answer. The code above seemed to indicate that it could be. It just
> > > > looks like old code that is no longer valid and never got removed.
> > > > Consequently this function has made it on my list of things to refactor
> > > > :^).
> > >
> > > Right, first_arg is only actually used for the implicit object argument,
> > > it's just easier to store it separately from the arguments in (). I'm
> > > not sure which code you mean is no longer valid?
> >
> > Yeah I agree that it's easier to store it separately.
> >
> > -- call.cc:build_over_call
> > `else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE) { tree arg =
> > build_this (first_arg != NULL_TREE ? first_arg : (*args)[arg_index]);`
> >
> > The trouble is, the code (shown above) does not assume that this holds
> > true. It handles the case where the implicit object argument was passed
> > in with t
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Just a quick addition here as I was starting to work on things I
realized where some misunderstandings were coming from. (Please also
see my previous e-mail, it is all still relevant, I just wanted to
clarify this.)
(From the other thread)
> > @@ -9949,7 +9951,8 @@ build_over_call (struct z_candidate cand, int flags,
> > tsubst_flags_t complain)
> > }
> > }
> > / Bypass access control for 'this' parameter. */
> > - else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE)
> > + else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
> > + || DECL_XOBJ_MEMBER_FUNC_P (fn))
>
>
> We don't want to take this path for xob fns. Instead I think we need to
> change the existing:
>
> > gcc_assert (first_arg == NULL_TREE);
>
>
> to assert that if first_arg is non-null, we're dealing with an xob fn,
> and then go ahead and do the same conversion as the loop body on first_arg.
(This thread)
> > Yeah, as I noted above I realized that just handling it the same way as
> > iobj member functions is fundamentally broken. I was staring at it last
> > night and eventually realized that I could just copy the loop body. I
> > ended up asserting in the body handling the implicit object argument
> > for xobj member functions that first_arg != NULL_TREE, which I wasn't
> > sure of, but it seems to work.
>
>
> That sounds like it might cause trouble with
>
> struct A {
> void f(this A);
> };
>
> int main()
> {
> (::f) (A());
> }
>
Upon reviewing your reply in the other thread, I noticed you are
maybe misunderstanding the assertion a little bit.
```
else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE)
{
/* SNIP */
if (first_arg != NULL_TREE)
first_arg = NULL_TREE;
else
++arg_index;
++i;
is_method = 1;
}
else if (DECL_XOBJ_MEMBER_FUNC_P (fn))
{
gcc_assert (cand->first_arg);
/* SNIP */
first_arg = NULL_TREE;
}
```
I already showed my code here but I didn't think to include the
previous conditional block to demonstrate how the first_arg variable
gets handled in it. Although, maybe you understood they set it to
NULL_TREE by the end of the block and I just poorly communicated that I
would be doing the same.
Perhaps you think it better to handle the implicit object argument
within the loop, but given that it is stored in first_arg I don't think
that would be correct. Granted, as I previously said, maybe there are
some situations where it isn't that I haven't encountered yet. The
other thing is there is some handling in the loop that isn't relevant
to the implicit object argument. Well, I would also just argue this
function needs a fair degree of refactoring, but I've already talked
about that.
Anyway, hopefully that clarifies things, and hopefully things actually
needed to be clarified and I'm not being a fool here. :) That's all I
had to add about this, back to work now.
Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> [combined reply to all three threads] > > On 11/9/23 23:24, waffl3x wrote: > > > > > I'm unfortunately going down a rabbit hole again. > > > > > > > > --function.h:608 > > > > `/* If pointers to member functions use the least significant bit to > > > > indicate whether a function is virtual, ensure a pointer to this > > > > function will have that bit clear. */ #define MINIMUM_METHOD_BOUNDARY > > > > ((TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn) > > > > ? MAX (FUNCTION_BOUNDARY, 2 * BITS_PER_UNIT) : FUNCTION_BOUNDARY)` > > > > > > So yes, it was for PMFs using the low bit of the pointer to indicate a > > > virtual member function. Since an xob memfn can't be virtual, it's > > > correct for them to have the same alignment as a static memfn. > > > > Is it worth considering whether we want to support virtual xobj member > > functions in the future? If that were the case would it be better if we > > aligned things a little differently here? Or might it be better if we > > wanted to support it as an extension to just effectively translate the > > declaration back to one that is a METHOD_TYPE? I imagine this would be > > the best solution for non-standard support of the syntax. We would > > simply have to forbid by-value and conversion semantics and on the > > user's side they would get consistent syntax. > > > > However, this flies in the face of the defective/contradictory spec for > > virtual function overrides. So I'm not really sure whether we would > > want to do this. I just want to raise the question before we lock in > > the alignment, if pushing the patch locks it in that is, I'm not really > > sure if it needs to be stable or not. > > > It doesn't need to be stable; we can increase the alignment of decls as > needed in new code without breaking older code. Okay great, good to know, it seems so obvious when you put it that way. > > > > All tests seemed to pass when applied to GCC14, but the results did > > > > something funny where it said tests disappeared and new tests appeared > > > > and passed. The ones that disappeared and the new ones that appeared > > > > looked like they were identical so I'm not worrying about it. Just > > > > mentioning it in case this is something I do need to look into. > > > > > > That doesn't sound like a problem, but I'm curious about the specific > > > output you're seeing. > > > > I've attached a few test result comparisons so you can take a look. > > > Looks like you're comparing results from different build directories and > the libitm test wrongly includes the build directory in the test "name". > So yeah, just noise. AH okay that makes sense. > > Side note, would you prefer I compile the lambda and by-value fixes > > into a new version of this patch? Or as a separate patch? Originally I > > had planned to put it in another patch, but I identified that the code > > I wrote in build_over_call was kind of fundamentally broken and it was > > almost merely coincidence that it worked at all. In light of this and > > your comments (which I've skimmed, I will respond directly below) I > > think I should just revise this patch with everything else. > > > Agreed. Will do then. > > > > There are a few known issues still present in this patch. Most > > > > importantly, > > > > the implicit object argument fails to convert when passed to by-value > > > > xobj > > > > parameters. This occurs both for xobj parameters that match the > > > > argument type > > > > and xobj parameters that are unrelated to the object type, but have > > > > valid > > > > conversions available. This behavior can be observed in the > > > > explicit-obj-by-value[1-3].C tests. The implicit object argument > > > > appears to be > > > > simply reinterpreted instead of any conversion applied. This is > > > > elaborated on > > > > in the test cases. > > > > > > Yes, that's because of: > > > > > > > @@ -9949,7 +9951,8 @@ build_over_call (struct z_candidate cand, int > > > > flags, tsubst_flags_t complain) > > > > } > > > > } > > > > / Bypass access control for 'this' parameter. */ > > > > - else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE) > > > > + else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE > > > > + || DECL_XOBJ_MEMBER_FUNC_P (fn)) > &g
Re: [PATCH v4 2/2] c++: Diagnostics for P0847R7 (Deducing this) [PR102609]
> > I had wanted to write about some of my frustrations with trying to
> > write a test for virtual specifiers and errors/warnings for
> > shadowing/overloading virtual functions, but I am a bit too tired at
> > the moment and I don't want to delay getting this up for another night.
> > In short, the standard does not properly specify the criteria for
> > overriding functions, which leaves a lot of ambiguity in how exactly we
> > should be handling these cases.
>
>
> Agreed, this issue came up in the C++ committee meeting today. See
>
> https://cplusplus.github.io/CWG/issues/2553.html
> https://cplusplus.github.io/CWG/issues/2554.html
>
> for draft changes to clarify some of these issues.
Ah I guess I should have read all your responses before sending any
back instead of going one by one. I ended up writing about this again I
think.
struct B {
virtual void f() const {}
};
struct S0 : B {
void f() {}
};
struct S1 : B {
void f(this S1&) {}
};
I had a bit of a debate with a peer, I initially disagreed with the
standard resolution because it disallows S1's declaration of f, while
S0's is an overload. I won't bore you with all the details of going
back and forth about the proposed wording, my feelings are mostly that
being able to overload something with an iobj member function but not
being able to with an xobj member function was inconsistent. He argued
that keeping restrictions high at first and lowering them later is
better, and overloading virtual functions is already not something you
should really ever do, so he was in favor of the proposed wording.
In light of our debate, my stance is that we should implement things as
per the proposed wording.
struct B {
virtual void foo() const&;
};
struct D : B {
void foo(this int);
};
This would be ill-formed now according to the change in wording. My
laymans interpretation of the semantics are that, the parameter list is
the same when the xobj parameter is ignore, so it overrides. And since
it overrides, and xobj member functions are not allowed to override, it
is an error.
To be honest, I still don't understand where the wording accounts for
the qualifiers of B::f, but my friend assured me that it is.
> > The standard also really poorly
> > specifies things related to the implicit object parameter and implicit
> > object argument which also causes some trouble. Anyhow, for the time
> > being I am not including my test for diagnostics related to a virtual
> > specifier on xobj member functions. I can't get it to a point I am
> > happy with it and I think there will need to be some discussion on how
> > exactly we want to handle that.
>
>
> The discussion might be easier with the testcase to refer to?
I'm pretty clear on how to proceed now. My biggest issue with the tests
is I didn't know what combination of errors to emit, whether to warn
for overriding, etc. There is still a bit of an issue on how to emit
errors for virtual specifiers on xobj member functions, it gets noisy
quickly, but maybe we do just emit all of them? This aside, what should
be done here is clear now.
Somewhat related is some warnings I wanted to implement for impossible
to call by-value xobj member functions. Ones that involve an unrelated
type get dicey because people could in theory have legitimate use cases
for that, P0847R7 includes an example of this combining recursive
lambdas and the overload pattern to create a visitor. However, I think
it would be reasonable to warn when a by-value xobj member function can
not be called due to the presence of overloads that take references.
Off the top of my head I don't recall how many overloads it would take
to prevent a by-value version from being called, nor have I looked into
how to implement this. But I do know that redeclarations of xobj member
functions as iobj member functions (and vice-versa) are not properly
identified when ref qualifiers are omitted from the corresponding (is
this the correct term here?) iobj member function.
> > I was fairly lazy with the changelog and commit message in this patch
> > as I expect to need to do another round on this patch before it can be
> > accepted. One specific question I have is whether I should be listing
> > out all the diagnostics that were added to a function. For the cases
> > where there were only one diagnostic added I stated it, but for
> > grokdeclarator which has the majority of the diagnostics I did not. I
> > welcome input here, really I request it, because the changelogs are
> > still fairly difficult for me to write. Hell, the commit messages are
> > hard to write, I feel I went overboard on the first patch but I guess
> > it's a fairly large patch so maybe it's alright? Again, I am looking
> > for feedback here if anyone is willing to provide it.
>
>
> ChangeLog entries are very brief summaries of the changes, there's
> absolutely no need to enumerate multiple diagnostics. If someone wants
> more detail they can look at the patch.
Noted, thank
Re: [PATCH v4 1/2] c++: Initial support for P0847R7 (Deducing this) [PR102609]
Ahh, I should have updated my progress last night after all, it would have saved us some time. Regardless, it's nice to see we independently came to the same conclusions. Side note, would you prefer I compile the lambda and by-value fixes into a new version of this patch? Or as a separate patch? Originally I had planned to put it in another patch, but I identified that the code I wrote in build_over_call was kind of fundamentally broken and it was almost merely coincidence that it worked at all. In light of this and your comments (which I've skimmed, I will respond directly below) I think I should just revise this patch with everything else. On Thursday, November 9th, 2023 at 2:53 PM, Jason Merrill wrote: > > > On 11/5/23 10:06, waffl3x wrote: > > > Bootstrapped and tested on x86_64-linux with no regressions. > > > > I originally threw this e-mail together last night, but threw in the > > towel when I thought I saw tests failing and went to sleep. I did a > > proper bootstrap and comparison and whatnot and found that there were > > thankfully no regressions. > > > > Anyhow, the first patch feels ready for trunk, the second needs at > > least one review, I'll write more on that in the second e-mail though. > > I put quite a lot into the commit message, in hindsight I think I may > > have gone overboard, but that isn't something I'm going to rewrite at > > the moment. I really want to get these patches up for review so they > > can be finalized. > > > > I'm also including my usual musings on things that came up as I was > > polishing off the patches. I reckon some of them aren't all that > > important right now but I would rather toss them in here than forget > > about them. > > > > I'm starting to think that we should have a general macro that > > indicates whether an implicit object argument should be passed in the > > call. It might be more clear than what is currently present. I've also > > noticed that there's a fair amount of places where instead of using > > DECL_NONSTATIC_MEMBER_FUNCTION_P the code checks if tree_code of the > > type is a METHOD_TYPE, which is exactly what the aforementioned macro > > does. > > > Agreed. > > > In build_min_non_dep_op_overload I reversed the branches of a condition > > because it made more sense with METHOD_TYPE first so it doesn't have to > > take xobj member functions into account on both branches. I am slightly > > concerned that flipping the branch around might have consequences, > > hence why I am mentioning it. Realistically I think it's probably fine > > though. > > > Agreed. Great, I was definitely concerned about this. > > BTW let me know if there's anything you would prefer to be done > > differently in the changelog, I am still having trouble writing them > > and I'm usually uncertain if I'm writing them properly. > > > (DECL_FUNCTION_XOBJ_FLAG): Define. > > > This is usually "New macro" or just "New". > > > * decl.cc (grokfndecl): New param XOBJ_FUNC_P, for xobj member > > functions set DECL_FUNCTION_XOBJ_FLAG and don't set > > DECL_STATIC_FUNCTION_P. > > (grokdeclarator): Check for xobj param, clear it's purpose and set > > is_xobj_member_function if it is present. When flag set, don't change > > type to METHOD_TYPE, keep it as FUNCTION_TYPE. > > Adjust call to grokfndecl, pass is_xobj_member_function. > > > These could be less verbose; for grokfndecl it makes sense to mention > the new parameter, but otherwise just saying "handle explicit object > member functions" is enough. Will do. > > It needs to be noted that we can not add checking for xobj member functions > > to > > DECL_NONSTATIC_MEMBER_FUNCTION_P as it is used in cp-objcp-common.cc. While > > it > > most likely would be fine, it's possible it could have unintended effects. > > In > > light of this, we will most likely need to do some refactoring, possibly > > renaming and replacing it. In contrast, DECL_FUNCTION_MEMBER_P is not used > > outside of C++ code, so we can add checking for xobj member functions to it > > without any concerns. > > > I think DECL_NONSTATIC_MEMBER_FUNCTION_P should probably be renamed to > DECL_IOBJ_MEMBER_FUNC_P to parallel the new macro... > > > @@ -3660,6 +3660,7 @@ build_min_non_dep_op_overload (enum tree_code op, > > > > expected_nargs = cp_tree_code_length (op); > > if (TREE_CODE (TREE_TYPE (overload)) == METHOD_TYPE > > + || DECL_XOBJ_MEMBER_FUNC_P (overload) > > > ...and then the combination should have its own macro, perhaps > DECL_OBJECT_MEMBER_FUNC_P,
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> > I'm unfortunately going down a rabbit hole again.
> >
> > --function.h:608
> > `/* If pointers to member functions use the least significant bit to
> > indicate whether a function is virtual, ensure a pointer to this function
> > will have that bit clear. */ #define MINIMUM_METHOD_BOUNDARY \\
> > ((TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn) \\ ? MAX
> > (FUNCTION_BOUNDARY, 2 * BITS_PER_UNIT) : FUNCTION_BOUNDARY)`
>
>
> So yes, it was for PMFs using the low bit of the pointer to indicate a
> virtual member function. Since an xob memfn can't be virtual, it's
> correct for them to have the same alignment as a static memfn.
Is it worth considering whether we want to support virtual xobj member
functions in the future? If that were the case would it be better if we
aligned things a little differently here? Or might it be better if we
wanted to support it as an extension to just effectively translate the
declaration back to one that is a METHOD_TYPE? I imagine this would be
the best solution for non-standard support of the syntax. We would
simply have to forbid by-value and conversion semantics and on the
user's side they would get consistent syntax.
However, this flies in the face of the defective/contradictory spec for
virtual function overrides. So I'm not really sure whether we would
want to do this. I just want to raise the question before we lock in
the alignment, if pushing the patch locks it in that is, I'm not really
sure if it needs to be stable or not.
> > I stumbled upon this while cleaning up the patch, grokfndecl is just so
> > full of cruft it's crazy hard to reason about. There's more than one
> > block that I am near certain is completely dead code. I would like to
> > just ignore them for now but some of them unfortunately pertain to xobj
> > functions. I just don't feel good about putting in any hacks, but to
> > really get any modifications in here correct it would need to be
> > refactored much more than I should be doing in this patch.
> >
> > Here's another example that I'm not sure how I want to address it.
> >
> > :10331~decl.cc grokfndecl
> > `int staticp = ctype && TREE_CODE (type) == FUNCTION_TYPE;`
> > :10506~decl.cc grokfndecl
> > `/* If this decl has namespace scope, set that up. */ if (in_namespace)
> > set_decl_namespace (decl, in_namespace, friendp); else if (ctype)
> > DECL_CONTEXT (decl) = ctype; else DECL_CONTEXT (decl) = FROB_CONTEXT
> > (current_decl_namespace ());`
> > And just a few lines down;
> > :10529~decl.cc
> > `/* Should probably propagate const out from type to decl I bet (mrs). */
> > if (staticp) { DECL_STATIC_FUNCTION_P (decl) = 1; DECL_CONTEXT (decl) =
> > ctype; }`
> >
> > If staticp is true, ctype must have been non-null, and if ctype is
> > non-null, the context for decl should have been set in the second
> > block. So why was the code in the second block added?
> >
> > commit f3665bdc1799c0421490b5e655f977570354
> > Author: Nathan Sidwell nat...@acm.org
> > Date: Tue Jul 28 08:57:36 2020 -0700
> >
> > c++: Set more DECL_CONTEXTs
> >
> > I discovered we were not setting DECL_CONTEXT in a few cases, and
> > grokfndecl's control flow wasn't making it clear that we were doing it
> > in all cases.
> >
> > gcc/cp/
> > * cp-gimplify.c (cp_genericize_r): Set IMPORTED_DECL's context.
> > * cp-objcp-common.c (cp_pushdecl): Set decl's context.
> > * decl.c (grokfndecl): Make DECL_CONTEXT setting clearer.
> >
> > According to the commit, it was because it was not clear, which quite
> > frankly I can agree to, it just wasn't determined that the code below
> > is redundantly setting the context so it wasn't removed.
> >
> > This puts me in a dilemma though, do I put another condition in that
> > code block for the xobj case even though the code is nearly dead? Or do
> > I give it a minor refactor for it to make a little more sense? If I add
> > to the code I feel like it's just going to add to the problem, while if
> > I give it a minor refactor it still won't look great and has a greater
> > chance of breaking something.
> >
> > In this case I'm going to risk refactoring it, staticp is only used in
> > that 1 place so I will just rip it out. I am not concerned with decl's
> > type spontaneously changing to something that is not FUNCTION_TYPE, and
> > if it did I think there are bigger problems afoot.
> >
> > I guess I'll know if I went too far with the refactoring when the patch
> > reaches you, do let me know about this one specifically though because
> > it took up a lot of my time trying to decide how to address it.
>
>
> Removing the redundant DECL_CONTEXT setting seems appropriate, and
> changing how staticp is handled to reflect that xobfns can also have
> FUNCTION_TYPE.
I removed static_p as it was only used in that one case, I'm pretty
happy with the resulting code but I saw you replied on the patch as
well so I'll see if you commented on it in the review and address your
thoughts there.
> > All tests
Re: [PATCH v4 1/2] c++: Initial support for P0847R7 (Deducing this) [PR102609]
I guess I'll be attaching all new e-mails here.
I found a new, kinda scary issue.
```
bool
start_preparsed_function (tree decl1, tree attrs, int flags)
{
tree ctype = NULL_TREE;
bool doing_friend = false;
/* Sanity check. */
gcc_assert (VOID_TYPE_P (TREE_VALUE (void_list_node)));
gcc_assert (TREE_CHAIN (void_list_node) == NULL_TREE);
tree fntype = TREE_TYPE (decl1);
if (TREE_CODE (fntype) == METHOD_TYPE)
ctype = TYPE_METHOD_BASETYPE (fntype);
else if (DECL_XOBJ_MEMBER_FUNC_P (decl1))
ctype = DECL_CONTEXT (decl1);
```
```
if (ctype && !doing_friend && !DECL_STATIC_FUNCTION_P (decl1))
{
/* We know that this was set up by `grokclassfn'. We do not
wait until `store_parm_decls', since evil parse errors may
never get us to that point. Here we keep the consistency
between `current_class_type' and `current_class_ptr'. */
tree t = DECL_ARGUMENTS (decl1);
gcc_assert (t != NULL_TREE && TREE_CODE (t) == PARM_DECL);
gcc_assert (TYPE_PTR_P (TREE_TYPE (t))
|| DECL_XOBJ_MEMBER_FUNC_P (decl1));
cp_function_chain->x_current_class_ref
= cp_build_fold_indirect_ref (t);
/* Set this second to avoid shortcut in cp_build_indirect_ref. */
cp_function_chain->x_current_class_ptr = t;
/* Constructors and destructors need to know whether they're "in
charge" of initializing virtual base classes. */
/* SNIP IRRELEVANT */
}
```
I made changes in this function, which suddenly sent execution into the
second code block. It seems like this would have been being bypassed
until the fix at the top of the function. Initially this was to fix a
problem with lambdas, but suddenly a lot of other stuff seems to be
breaking. I haven't run the tests yet but... I have a really bad
feeling about this.
So my concerns here are, one, this seems kind of important upon looking
at it, what kind of stuff might have been broken when this was being
bypassed that I didn't notice? And two, how in the world was it working
when this was being bypassed?
I have a hunch that some of the reinterpretation and "just works"
behavior might have had something to do with this block of code being
bypassed. I also suspect that this area will need some changes to make
by-value xobj parameters work. However, I'm a little confused at why
this block is necessary at all. Like I have noted before, when
attempting to call a by-value xobj member function, if there are no
viable conversions, the call will fail. So it's checking for that
somewhere.
normal.C: In explicit object member function 'uintptr_t S::f(this uintptr_t)':
normal.C:15:33: error: invalid type argument (have 'uintptr_t' {aka 'long
unsigned int'})
15 | uintptr_t f(this uintptr_t n) {
| ^
normal.C: In explicit object member function 'uintptr_t S::g(this FromS)':
normal.C:18:34: error: invalid type argument (have 'FromS')
18 | uintptr_t g(this FromS from_s) {
| ^
But now that we are entering this code block, (when compiling
explicit-obj-by-value2.C) these errors are popping up. Why now? Why
isn't this being handled in the same place other things are? How
important is this block of code really? Is this the origin of the weird
errors where rvalue refs are being accepted for functions that take
const rvalue refs?
Is this code just setting up the 'this' pointer? I have so many guesses
and so many questions. I don't think I can just bypass it though, but
maybe I can? This is another one that feels really deep down the rabbit
hole so I would appreciate any insight that can be provided.
Anyway, I had thought that I probably just need to change how
build_over_call works to get passing to by-value xobj params to work.
But now that I've found this code and gotten these new errors to
surprise me, now I'm a little worried.
If it's just for the 'this' pointer then it's probably fine. I need to
sleep now, I hadn't planned on looking at this for as long as I did but
I got sucked in. I think tomorrow I will go back to bypassing this code
block and try to make changes to build_over_call works and see if that
does the trick. But things feel all over the place now so I'm a little
concerned about what else I might be neglecting.
Thanks,
Alex
Re: [PATCH v4 1/2] c++: Initial support for P0847R7 (Deducing this) [PR102609]
I noticed I made a bit of a mistake in grokdeclarator:find_xobj_parm, this code: ``` if (!parm_list || parm_list == void_list_node) return false; if (TREE_PURPOSE (parm_list) != this_identifier) return false; ``` Can be simplified to this code: ``` if (!parm_list || TREE_PURPOSE (parm_list) != this_identifier) return false; ``` While working on lambda support I found that I need to find the xobj parameter before we get to grokdeclarator, so I was going to reuse the same code. After looking at it I realized, hey, I wonder what void_list_node's purpose field holds. So I checked, and found that tree.cc:build_common_tree_nodes initializes void_list_node with a NULL_TREE for it's purpose field. It's obviously not going to be a big deal but simpler code is better. I will most likely fix this if I have time later just so the code doesn't look semantically different in the lambda support patch. Alex
[PATCH v4 2/2] c++: Diagnostics for P0847R7 (Deducing this) [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions. Finally, the fabled diagnostics patch. I would like to note really quickly that there was never a v2 and v3 of this patch, only the first of these 2 had those versions. Originally I had planned to revise this patch alongside the first but it just didn't happen. Anyhow, I decided to match the version of this second patch to the current first patch to avoid any confusion. With that out of the way, I feel mostly okay about the code in this patch, but I have a feeling it will need a revision, especially with the large amounts of comments I left in. At the very least I expect to need to pull those out before the patch can be accepted. I had wanted to write about some of my frustrations with trying to write a test for virtual specifiers and errors/warnings for shadowing/overloading virtual functions, but I am a bit too tired at the moment and I don't want to delay getting this up for another night. In short, the standard does not properly specify the criteria for overriding functions, which leaves a lot of ambiguity in how exactly we should be handling these cases. The standard also really poorly specifies things related to the implicit object parameter and implicit object argument which also causes some trouble. Anyhow, for the time being I am not including my test for diagnostics related to a virtual specifier on xobj member functions. I can't get it to a point I am happy with it and I think there will need to be some discussion on how exactly we want to handle that. I was fairly lazy with the changelog and commit message in this patch as I expect to need to do another round on this patch before it can be accepted. One specific question I have is whether I should be listing out all the diagnostics that were added to a function. For the cases where there were only one diagnostic added I stated it, but for grokdeclarator which has the majority of the diagnostics I did not. I welcome input here, really I request it, because the changelogs are still fairly difficult for me to write. Hell, the commit messages are hard to write, I feel I went overboard on the first patch but I guess it's a fairly large patch so maybe it's alright? Again, I am looking for feedback here if anyone is willing to provide it. I've written more than I want here, so I'll wrap this e-mail up and go to bed. I am very happy to be getting close to a final product here. Hopefully if all goes well I'll be able to fit in the final missing features before feature lock hits. AlexFrom c8e8155a635fab7f326d0ad32326da352d7c323e Mon Sep 17 00:00:00 2001 From: waffl3x Date: Sun, 5 Nov 2023 05:17:18 -0700 Subject: [PATCH 2/2] c++: Diagnostics for C++23 P0847R7 (Deducing this) [PR102609] This patch adds diagnostics for various ill-formed code related to xobj member functions. Some of the code in here leaves something to be desired, but the majority of cases should be handled. I opted to add a new TFF flag despite only using it in a single place, other solutions seemed non ideal and there are plenty of bits left. Some of the diagnostics are more scattered around than I would like, perhaps this could be refactored in the future, especially those in grokfndecl. PR c++/102609 gcc/cp/ChangeLog: PR c++/102609 Diagnostics for C++23 P0847R7 - Deducing this. * cp-tree.h (TFF_XOBJ_FUNC): Define. * decl.cc (grokfndecl): Diagnose cvref-qualifiers on an xobj member function. (grokdeclarator): Diagnostics * error.cc (dump_function_decl): For xobj member function add TFF_XOBJ_FUNC bit to dump_parameters flags argument. (dump_parameters): When printing xobj member function's params add "this" to the first param. (function_category): Say so when in an xobj member function. * parser.cc (cp_parser_decl_specifier_seq): Diagnose incorrectly positioned "this" specifier. (cp_parser_parameter_declaration): Diagnose default argument on xobj params. * semantics.cc (finish_this_expr): Diagnose uses of "this" in body of xobj member function. gcc/testsuite/ChangeLog: PR c++/102609 Diagnostics for C++23 P0847R7 - Deducing this. * g++.dg/cpp23/explicit-obj-cxx-dialect-A.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-B.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-C.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-D.C: New test. * g++.dg/cpp23/explicit-obj-cxx-dialect-E.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics1.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics2.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics3.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics4.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics5.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics6.C: New test. * g++.dg/cpp23/explicit-obj-diagnostics7.C: New test. Signed-off-by: waffl3x --- gcc/cp/cp-tree.h | 5 +- gcc/cp/decl.cc| 133 ++--- gcc/cp/error.cc
[PATCH v4 1/2] c++: Initial support for P0847R7 (Deducing this) [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions. I originally threw this e-mail together last night, but threw in the towel when I thought I saw tests failing and went to sleep. I did a proper bootstrap and comparison and whatnot and found that there were thankfully no regressions. Anyhow, the first patch feels ready for trunk, the second needs at least one review, I'll write more on that in the second e-mail though. I put quite a lot into the commit message, in hindsight I think I may have gone overboard, but that isn't something I'm going to rewrite at the moment. I really want to get these patches up for review so they can be finalized. I'm also including my usual musings on things that came up as I was polishing off the patches. I reckon some of them aren't all that important right now but I would rather toss them in here than forget about them. I'm starting to think that we should have a general macro that indicates whether an implicit object argument should be passed in the call. It might be more clear than what is currently present. I've also noticed that there's a fair amount of places where instead of using DECL_NONSTATIC_MEMBER_FUNCTION_P the code checks if tree_code of the type is a METHOD_TYPE, which is exactly what the aforementioned macro does. In build_min_non_dep_op_overload I reversed the branches of a condition because it made more sense with METHOD_TYPE first so it doesn't have to take xobj member functions into account on both branches. I am slightly concerned that flipping the branch around might have consequences, hence why I am mentioning it. Realistically I think it's probably fine though. I have a test prepared for diagnosing virtual specifiers on xobj member functions, but it's got some issues so I won't be including it with the following diagnostic patch. Diagnostics for virtual specifiers are still implemented, it's just the test that is having trouble. I mostly had a hard time working out edge cases, and the standard doesn't actually properly specify what the criteria for overriding a function is so I've been stumped on what behavior I want it to have. So for the time being, it only diagnoses uses of virtual on xobj member functions, while errors for final and override are handled by code that is already present. This can result in multiple errors, but again, I don't know how I want to handle it yet, especially since the standard doesn't specify this stuff very well. BTW let me know if there's anything you would prefer to be done differently in the changelog, I am still having trouble writing them and I'm usually uncertain if I'm writing them properly. Alex From e730dcba51503446cc362909fcab19361970b448 Mon Sep 17 00:00:00 2001 From: waffl3x Date: Sat, 4 Nov 2023 05:35:10 -0600 Subject: [PATCH 1/2] c++: Initial support for C++23 P0847R7 (Deducing this) [PR102609] This patch implements initial support for P0847R7 without diagnostics. My goal was to minimize changes to the existing code. To achieve this I chose to treat xobj member functions as static member functions, while opting into member function handling when necessary. This seemed to be the better choice since most of the time they are more like static member functions. This is achieved by inhibiting conversion of the declaration's type from FUNCTION_TYPE to METHOD_TYPE. Most if not everything seems to differentiate between member functions and static member functions by inspecting the FUNCTION_DECL's type, so forcing this is sufficient. An xobj member function is any member function that is declared with an xobj parameter as it's first parameter. This information is passed through the declarator's parameter list, stored in the purpose of the parameter's tree_list node. Normally this is used to store default arguments, but as default arguments are not allowed for xobj parameters it is fine for us to hijack it. By utilizing this we can pass this information from cp_parser_parameter_declaration over to grokdeclarator without adding anything new to the tree structs. We still need to differentiate this new function variety from static member functions and regular functions, and since this information needs to stick to the declaration we should select a more concrete place to store it. Unlike the previous hack for parameters, we instead add a flag to lang_decl_fn, the only modification this patch makes to any tree data-structures. We could probably try to stick the information in the decl's parameters somewhere, but I think a proper flag is justified. The new flag can be set and cleared through DECL_FUNCTION_XOBJ_FLAG, it is invalid to use this with anything other than FUNCTION_DECL nodes. For inspecting the value of this flag DECL_XOBJ_MEMBER_FUNC_P should be used, this macro is safe to use with any node type and will harmlessly evaluate to false for invalid node types. It needs to be noted that we can not add checking for xobj member functions to DECL_NONSTATIC_MEMBER_FUNCTION_P
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
I'm unfortunately going down a rabbit hole again.
--function.h:608
```
/* If pointers to member functions use the least significant bit to
indicate whether a function is virtual, ensure a pointer
to this function will have that bit clear. */
#define MINIMUM_METHOD_BOUNDARY \
((TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn) \
? MAX (FUNCTION_BOUNDARY, 2 * BITS_PER_UNIT) : FUNCTION_BOUNDARY)
```
--decl.cc:10400 grokfndecl
```
if (TREE_CODE (type) == METHOD_TYPE)
{
tree parm = build_this_parm (decl, type, quals);
DECL_CHAIN (parm) = parms;
parms = parm;
/* Allocate space to hold the vptr bit if needed. */
SET_DECL_ALIGN (decl, MINIMUM_METHOD_BOUNDARY);
}
```
The good news is I think I found where the alignment is being set, so
it can be addressed later if desired.
I stumbled upon this while cleaning up the patch, grokfndecl is just so
full of cruft it's crazy hard to reason about. There's more than one
block that I am near certain is completely dead code. I would like to
just ignore them for now but some of them unfortunately pertain to xobj
functions. I just don't feel good about putting in any hacks, but to
really get any modifications in here correct it would need to be
refactored much more than I should be doing in this patch.
Here's another example that I'm not sure how I want to address it.
~decl.cc:10331 grokfndecl
```
int staticp = ctype && TREE_CODE (type) == FUNCTION_TYPE;
```
~decl.cc:10506 grokfndecl
```
/* If this decl has namespace scope, set that up. */
if (in_namespace)
set_decl_namespace (decl, in_namespace, friendp);
else if (ctype)
DECL_CONTEXT (decl) = ctype;
else
DECL_CONTEXT (decl) = FROB_CONTEXT (current_decl_namespace ());
```
And just a few lines down;
~decl.cc:10529
```
/* Should probably propagate const out from type to decl I bet (mrs). */
if (staticp)
{
DECL_STATIC_FUNCTION_P (decl) = 1;
DECL_CONTEXT (decl) = ctype;
}
```
If staticp is true, ctype must have been non-null, and if ctype is
non-null, the context for decl should have been set in the second
block. So why was the code in the second block added?
commit f3665bdc1799c0421490b5e655f977570354
Author: Nathan Sidwell
Date: Tue Jul 28 08:57:36 2020 -0700
c++: Set more DECL_CONTEXTs
I discovered we were not setting DECL_CONTEXT in a few cases, and
grokfndecl's control flow wasn't making it clear that we were doing it
in all cases.
gcc/cp/
* cp-gimplify.c (cp_genericize_r): Set IMPORTED_DECL's context.
* cp-objcp-common.c (cp_pushdecl): Set decl's context.
* decl.c (grokfndecl): Make DECL_CONTEXT setting clearer.
According to the commit, it was because it was not clear, which quite
frankly I can agree to, it just wasn't determined that the code below
is redundantly setting the context so it wasn't removed.
This puts me in a dilemma though, do I put another condition in that
code block for the xobj case even though the code is nearly dead? Or do
I give it a minor refactor for it to make a little more sense? If I add
to the code I feel like it's just going to add to the problem, while if
I give it a minor refactor it still won't look great and has a greater
chance of breaking something.
In this case I'm going to risk refactoring it, staticp is only used in
that 1 place so I will just rip it out. I am not concerned with decl's
type spontaneously changing to something that is not FUNCTION_TYPE, and
if it did I think there are bigger problems afoot.
I guess I'll know if I went too far with the refactoring when the patch
reaches you, do let me know about this one specifically though because
it took up a lot of my time trying to decide how to address it.
> > The code doesn't seem to reflect that, perhaps since the underlying
> > node type is the same (as far as I can tell, they both inherit from
> > tree_type_non_common) it wasn't believed to be necessary.
>
>
> Curious. It might also be a remnant of how older code dealt with how
> sometimes a member function changes between FUNCTION_TYPE and
> METHOD_TYPE during parsing.
Sounds plausible.
> > Upon looking at DECL_CONTEXT though I see it seems you were thinking of
> > FUNCTION_DECL. I haven't observed DECL_CONTEXT being set for
> > FUNCTION_DECL nodes though, perhaps it should be? Although it's more
> > likely that it is being set and I just haven't noticed, but if that's
> > the case I'll have to make a note to make sure it is being set for xobj
> > member functions.
>
>
> I would certainly expect it to be getting set already.
This being on my mind is partially what sent me down the rabbit hole
above, but yeah, it seems like it is.
> > I was going to say that this seems like a redundancy, but I realized
> > that the type of a member function pointer is tied to the struct, so it
> > actually ends up relevant for METHOD_TYPE nodes. I would hazard a guess
> > that when
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> > That leaves 2, 4, and 5. > > > > 2. I am pretty sure xobj functions should have the struct they are a > > part of recorded as the method basetype member. I have already checked > > that function_type and method_type are the same node type under the > > hood and it does appear to be, so it should be trivial to set it. > > However I do have to wonder why static member functions don't set it, > > it seems to be that it would be convenient to use the same field. Can > > you provide any insight into that? > > > method basetype is only for METHOD_TYPE; if you want the containing type > of the function, that's DECL_CONTEXT. -- gcc/tree.h:530 #define FUNC_OR_METHOD_CHECK(T) TREE_CHECK2 (T, FUNCTION_TYPE, METHOD_TYPE) -- gcc/tree.h:2518 #define TYPE_METHOD_BASETYPE(NODE) \ (FUNC_OR_METHOD_CHECK (NODE)->type_non_common.maxval) The code doesn't seem to reflect that, perhaps since the underlying node type is the same (as far as I can tell, they both inherit from tree_type_non_common) it wasn't believed to be necessary. Upon looking at DECL_CONTEXT though I see it seems you were thinking of FUNCTION_DECL. I haven't observed DECL_CONTEXT being set for FUNCTION_DECL nodes though, perhaps it should be? Although it's more likely that it is being set and I just haven't noticed, but if that's the case I'll have to make a note to make sure it is being set for xobj member functions. I was going to say that this seems like a redundancy, but I realized that the type of a member function pointer is tied to the struct, so it actually ends up relevant for METHOD_TYPE nodes. I would hazard a guess that when forming member function pointers the FUNCTION_DECL node was not as easily accessed. If I remember correctly that is not the case right now so it might be worthwhile to refactor away from TYPE_METHOD_BASETYPE and replace uses of it with DECL_CONTEXT. I'm getting ahead of myself though, I'll stop here and avoid going on too much of a refactoring tangent. I do want this patch to make it into GCC14 after all. > > 4. I have no comment here, but it does concern me since I don't > > understand it at all. > > > In the list near the top of cp-tree.h, DECL_LANG_FLAG_6 for a > FUNCTION_DECL is documented to be DECL_THIS_STATIC, which should only be > set on the static member. Right, I'll try to remember to check this area in the future, but yeah that tracks because I did remove that flag. Removing that flag just so happened to be the start of this saga of bug fixes but alas, it had to be done. > > 5. I am pretty sure this is fine for now, but if xobj member functions > > ever were to support virtual/override capabilities, then it would be a > > problem. Is my understanding correct, or is there some other reason > > that iobj member functions have a different value here? > > > It is surprising that an iobj memfn would have a different DECL_ALIGN, > but it shouldn't be a problem; the vtables only rely on alignment being > at least 2. I'll put a note for myself to look into it in the future, it's an oddity at minimum and oddities interest me :^). > > There are also some differences in the arg param in > > cp_build_addr_expr_1 that concerns me, but most of them are reflected > > in the differences I have already noted. I had wanted to include these > > differences as well but I have been spending too much time staring at > > it, it's no longer productive. In short, the indirect_ref node for xobj > > member functions has reference_to_this set, while iobj member functions > > do not. > > > That's a result of difference 3. Okay, makes sense, I'm mildly concerned about any possible side effects this might have since we have a function_type node suddenly going through execution paths that only method_type went through before. The whole "reference_to_this" "pointer_to_this" thing is a little confusing because I'm pretty sure that doesn't refer to the actual `this` object argument or parameter since I've seen it all over the place. Hopefully it's benign. > > The baselink binfo field has the private flag set for xobj > > member functions, iobj member functions does not. > > > TREE_PRIVATE on a binfo is part of BINFO_ACCESS, which isn't a fixed > value, but gets updated during member search. Perhaps the differences > in consideration of conversion to a base led to it being set or cleared > differently? I wouldn't worry too much about it unless you see > differences in access control. Unfortunately I don't have any tests for private/public access yet, it's one of the area's I've neglected. Unfortunately I probably won't put too much effort into writing TOO many more right now as it takes up a lot of my time. I still have to clean up the ones I currently have and I have a few I wanted to write that are not yet written. > > I've spent too much time on this write up, so I am calling it here, it > > wasn't all a waste of time because half of what I was doing here are > > things I was going to need to do
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
The problem with operators is fixed, now starts a long period of testing. It's been so long since I've gotten to this part that I almost forgot that I have to do it :'). When/if regtests and bootstrap all pass I will format the patch and submit it. --- Original Message --- On Wednesday, November 1st, 2023 at 5:15 PM, waffl3x wrote: > > > Just want to quickly check, when is the cutoff for GCC14 exactly? I > want to know how much time I have left to try to tackle this bug with > subscript. I'm going to be crunching out final stuff this week and I'll > try to get a (probably non-final) patch for you to review today. > > Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Just want to quickly check, when is the cutoff for GCC14 exactly? I want to know how much time I have left to try to tackle this bug with subscript. I'm going to be crunching out final stuff this week and I'll try to get a (probably non-final) patch for you to review today. Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
I woke up today and figured it out in about 30 minutes, I don't know if
this solution would be okay but I am running away from
cp_build_addr_expr_1 for now. I think this is another place I will have
the urge to refactor in the future, but if I keep looking at it right
now I am just going to waste all my energy before I finish everything
else.
This line
```
else if (BASELINK_P (TREE_OPERAND (arg, 1)))
```
near the bottom of the function is almost certainly dead. The switch
case (near the top) for baselink reassigns arg.
```
case BASELINK:
arg = BASELINK_FUNCTIONS (arg);
```
And I'm pretty sure a baselink's functions can't be a baselink, so that
case near the bottom is almost certainly dead. I've put it in my todo
so I will look at it in the future.
Anyway, here is my new solution, it seems to work, but as indicated in
the comment (for myself) I'm not sure this handles constexpr things
right as near the bottom of this function there is some constexpr
handling. I know I said it took me 30 minutes but I realized I had some
holes I needed to check as I was writing this email, so it was more
like 2 hours.
```
/* Forming a pointer-to-member is a use of non-pure-virtual fns. */
if (TREE_CODE (t) == FUNCTION_DECL
&& !DECL_PURE_VIRTUAL_P (t)
&& !mark_used (t, complain) && !(complain & tf_error))
return error_mark_node;
/* Exception for non-overloaded explicit object member function. */
/* I am pretty sure this is still not perfect, I think we aren't
handling some constexpr stuff, but I am leaving it for now. */
if (TREE_CODE (TREE_TYPE (t)) == FUNCTION_TYPE)
return build_address (t);
type = build_ptrmem_type (context_for_name_lookup (t),
TREE_TYPE (t));
t = make_ptrmem_cst (type, t);
return t;
}
```
I'm sharing it because I am still uncertain on whether this is the
ideal solution, this function is kind of a mess in general (sorry) so
it's hard to tell.
There's still a few things in my previous email that I would like
looked at, primarily the differences I observed in returns from
grokdeclarator, but I am glad to finally be moving forward on to other
things. Hopefully I can put together the patches with this one fixed.
(Of course I still have to fix the other bug I found as well.)
I had thought I might try to get lambda support and by value xobj
params working in this patch, but I think that will still be something
I work on once I am finished the initial patch. I still want to get
both of those into GCC14 though so I probably need to speed things up.
Alex
>
>
> I've been under the weather so I took a few days break, I honestly was
> also very reluctant to pick it back up. The current problem is what I
> like to call "not friendly", but I am back at it now.
>
> > > I don't understand what this means exactly, under what circumstances
> > > would find the member function. Oh, I guess while in the body of
> > > it's class, I hadn't considered that. Is that what you're referring to?
> >
> > Right:
> >
> > struct A {
> > void g(this A&);
> > A() {
> > ::g; // ok
> > // same error as for an implicit object member function
> > }
> > };
>
>
> I fully get this now, I threw together a test for it so this case
> doesn't get forgotten about. Unfortunately though, I am concerned that
> the approach I was going to take to fix the crash would have the
> incorrect behavior for this.
>
> Here is what I added to cp_build_addr_expr_1 with context included.
> `case OFFSET_REF: offset_ref: /* Turn a reference to a non-static data member
> into a pointer-to-member. */ { tree type; tree t; gcc_assert (PTRMEM_OK_P
> (arg)); t = TREE_OPERAND (arg, 1); if (TYPE_REF_P (TREE_TYPE (t))) { if
> (complain & tf_error) error_at (loc, "cannot create pointer to reference
> member %qD", t); return error_mark_node; } /* -- Waffl3x additions start --
> */ /* Exception for non-overloaded explicit object member function. */ if
> (TREE_CODE (TREE_TYPE (t)) == FUNCTION_TYPE) return build1 (ADDR_EXPR,
> unknown_type_node, arg); /* -- Waffl3x additions end -- */ /* Forming a
> pointer-to-member is a use of non-pure-virtual fns. */ if (TREE_CODE (t) ==
> FUNCTION_DECL && !DECL_PURE_VIRTUAL_P (t) && !mark_used (t, complain) &&
> !(complain & tf_error)) return error_mark_node;`
>
> I had hoped this naive solution would work just fine, but unfortunately
> the following code fails to compile with an error.
>
> `struct S { void f(this S&) {} }; int main() { void (*a)(S&) = ::f; }`
> normal_static.C: In function ‘int main()’:
> normal_static.C:13:25: error: cannot convert ‘S::f’ from type ‘void(S&
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
I've been under the weather so I took a few days break, I honestly was
also very reluctant to pick it back up. The current problem is what I
like to call "not friendly", but I am back at it now.
> > I don't understand what this means exactly, under what circumstances
> > would find the member function. Oh, I guess while in the body of
> > it's class, I hadn't considered that. Is that what you're referring to?
>
>
> Right:
>
> struct A {
> void g(this A&);
> A() {
> ::g; // ok
> // same error as for an implicit object member function
> }
> };
I fully get this now, I threw together a test for it so this case
doesn't get forgotten about. Unfortunately though, I am concerned that
the approach I was going to take to fix the crash would have the
incorrect behavior for this.
Here is what I added to cp_build_addr_expr_1 with context included.
```
case OFFSET_REF:
offset_ref:
/* Turn a reference to a non-static data member into a
pointer-to-member. */
{
tree type;
tree t;
gcc_assert (PTRMEM_OK_P (arg));
t = TREE_OPERAND (arg, 1);
if (TYPE_REF_P (TREE_TYPE (t)))
{
if (complain & tf_error)
error_at (loc,
"cannot create pointer to reference member %qD", t);
return error_mark_node;
}
/* -- Waffl3x additions start -- */
/* Exception for non-overloaded explicit object member function. */
if (TREE_CODE (TREE_TYPE (t)) == FUNCTION_TYPE)
return build1 (ADDR_EXPR, unknown_type_node, arg);
/* -- Waffl3x additions end -- */
/* Forming a pointer-to-member is a use of non-pure-virtual fns. */
if (TREE_CODE (t) == FUNCTION_DECL
&& !DECL_PURE_VIRTUAL_P (t)
&& !mark_used (t, complain) && !(complain & tf_error))
return error_mark_node;
```
I had hoped this naive solution would work just fine, but unfortunately
the following code fails to compile with an error.
```
struct S {
void f(this S&) {}
};
int main() {
void (*a)(S&) = ::f;
}
```
normal_static.C: In function ‘int main()’:
normal_static.C:13:25: error: cannot convert ‘S::f’ from type ‘void(S&)’ to
type ‘void (*)(S&)’
13 | void (*a)(S&) = ::f;
| ^
So clearly it isn't going to be that easy. I went up and down looking
at how the single static case is handled, and tried to read the code in
build_ptrmem_type and build_ptrmemfunc_type but I had a hard time
figuring it out.
The good news is, this problem was difficult enough that it made me
pick a proper diff tool with regex support instead of using a silly web
browser tool and pasting things into it. Or worse, pasting them into a
tool and doing replacement and then pasting them into the silly web
browser tool. I have been forced to improve my workflow thanks to this
head scratcher. So it's not all for naught.
Back on topic, it's not really the optimization returning a baselink
that is causing the original crash. It's just the assert in
build_ptrmem_type failing when a FUNCTION_TYPE is reaching it. The
optimization did throw me for a loop when I was comparing how my
previous version (that incorrectly set the lang_decl_fn ::
static_function flag) was handling things. Looking back, I think I
explained myself and the methodology I was using to investigate really
poorly, I apologize for the confusion I might have caused :).
To state it plainly, it seems to me that the arg parameter being passed
into cp_build_addr_expr_1 for explicit object member functions is
(mostly) pretty much correct and what we would want.
So the whole thing with the baselink optimization was really just a red
herring that I was following. Now that I have a better understanding of
what's happening leading up to and in cp_build_addr_expr_1 I don't
think it's relevant at all for this problem. With that said, I am
questioning again if the optimization that returns a baselink node is
the right way to do things. So this is something I'm going to put down
into my little notes text file to investigate at a later time, and
forget about it for the moment as it shouldn't be causing any friction
for us here.
Anyway, as I eluded to above, if I can't figure out the right way to
solve this problem in a decent amount of time I think I'm going to
leave it for now. I'll come back to it once other higher priority
things are fixed or finished. And hopefully someone more familiar with
this area of the code will have a better idea of what we need to do to
handle this case in a non-intrusive manner.
That wraps up my current status on this specifically. But while
investigating it I uncovered a few things that I feel are important to
discuss/note.
I wanted to change DECL_NONSTATIC_MEMBER_FUNCTION_P to include explicit
object member functions, but it had some pr
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> >
> > Based on what you've said, I assume that OFFSET_REF handles static
> > member functions that are overloaded. But as I've said this seems to
> > contradict the comments I'm reading, so I'm not sure that I'm
> > understanding you correctly.
>
>
> That's right. For instance,
>
> struct A {
> static void g();
> static void g(int);
> };
>
> void (*p)(int) = ::g; // cp_build_addr_expr_1 gets an OFFSET_REF
>
> > > > I think we need the OFFSET_REF for an explicit-object member function
> > > > because it expresses that the code satisfies the requirement "If the
> > > > operand names an explicit object member function, the operand shall be a
> > > > qualified-id."
> >
> > I do agree here, but it does reinforce that OFFSET_REF is no longer
> > just for members represented by pointer to member type. So that might
> > be something to take into consideration.
>
>
> An OFFSET_REF that isn't type_unknown_p, agreed.
>
> > > > It might simplify things to remove the optimization in build_offset_ref
> > > > so we get an OFFSET_REF even for a single static member function, and
> > > > add support for that to cp_build_addr_expr_1.
> >
> > I don't think this should be necessary, the "right thing" should just
> > be done for explicit-object member functions. With all the stuff going
> > on here that I missed I'm starting to wonder how function overloads
> > ever worked at all in my patch. On the other hand though, this
> > optimization probably could be documented better, but I very well might
> > have missed it even if it were.
> >
> > Hell, maybe it needs a greater redesign altogether, it seems strange to
> > me to bundle overload information in with a construct for a specific
> > expression. (Assuming that's whats happening of course, I still don't
> > fully understand it.) It's not like this has rules unique to it for how
> > overload resolution is decided, right? Initializing a param/variable of
> > pointer to function type with an overloaded function resolves that with
> > similar rules, I think? Maybe it is a little different now that I write
> > it out loud.
> >
> > I wasn't going to finish my musings about that, but it made me realize
> > that it might not actually be correct for address of explicit-object
> > member functions to be wrapped by OFFSET_REF. I mean surely it's fine
> > because based on what you've said static member functions are also
> > wrapped by OFFSET_REF, so it's likely fully implemented, especially
> > considering things worked before. But now that there are 2 different
> > varieties of class members that the address of them can be taken, it
> > might make sense to split things up a bit? Then again, why were static
> > member functions ever handled the same way? Taking the address of other
> > static members isn't handled in the same way here is it?
>
>
> Functions are different because of overloading; in general we can't
> decide what an expression that names a function actually means until we
> have enough context to decide which function, exactly. So we represent
> the id-expression largely as lookup+syntax until overload resolution
> turns it into a specific function. The type_unknown_p check earlier in
> cp_build_addr_expr_1 is for that case.
Yeah this all makes sense, but that's why I was confused by the
following documentation from cp-tree.def.
```
/* An OFFSET_REF is used in two situations:
1. An expression of the form `A::m' where `A' is a class and `m' is
a non-static member. In this case, operand 0 will be a TYPE
(corresponding to `A') and operand 1 will be a FIELD_DECL,
BASELINK, or TEMPLATE_ID_EXPR (corresponding to `m').
The expression is a pointer-to-member if its address is taken,
but simply denotes a member of the object if its address is not
taken.
```
> An OFFSET_REF that isn't type_unknown_p, agreed.
But I suppose that's what this might have been referring to.
So is that the case then? OFFSET_REF might be for a regular address of
member expression unless it's type_unknown_p?
>
> An id-expression that names a single non-template function
> (!really_overloaded_fn) is handled somewhat differently, as we don't
> need to defer everything. But that means various special-case code.
>
> Currently build_offset_ref special-cases ::f for a single static
> member function, but we can't use the same special case for single
> explicit object member functions because we need to distinguish between
> ::f and somehow to check the requirement I quoted above. So it
> seems to me we'll need to add support for single explicit object member
> functions in the OFFSET_REF handling in cp_build_addr_expr_1. And I
> thought if we're doing that, perhaps we want to move the single static
> handling over there as well, but that's not necessary.
>
> Jason
I'm done for today but it does seem like that special case is what is
causing my crash. I found that it only jumps into the section that it
crashes in when there are no overloads. I'm
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> (waffl3x (me)) > At a glance it seems like all I need to do then is disable the > PTRMEM_OK_P flag then. I'm just now realizing that I'm almost certainly wrong about this. It still needs PTRMEM_OK_P set if there are any implicit-object member functions in the overload set. That is, if OFFSET_REF includes that information... but it doesn't seem like it does? Reading the information on OFFSET_REF, particularly build_offset_ref, seems to indicate that OFFSET_REF (at least historically) was only for things with a pointer to member type. > > An OFFSET_REF (with PTRMEM_OK_P) is used to express that we saw the > > ::f syntax, so we could build a pointer to member if it resolves to an > > implicit-object member function. > > > > For an overload set containing only a single static member function, > > build_offset_ref doesn't bother to build an OFFSET_REF, but returns the > > BASELINK itself. Based on what you've said, I assume that OFFSET_REF handles static member functions that are overloaded. But as I've said this seems to contradict the comments I'm reading, so I'm not sure that I'm understanding you correctly. I suppose that will be pretty easy to test, so I'll just do that as well. > > I think we need the OFFSET_REF for an explicit-object member function > > because it expresses that the code satisfies the requirement "If the > > operand names an explicit object member function, the operand shall be a > > qualified-id." I do agree here, but it does reinforce that OFFSET_REF is no longer just for members represented by pointer to member type. So that might be something to take into consideration. > > It might simplify things to remove the optimization in build_offset_ref > > so we get an OFFSET_REF even for a single static member function, and > > add support for that to cp_build_addr_expr_1. I don't think this should be necessary, the "right thing" should just be done for explicit-object member functions. With all the stuff going on here that I missed I'm starting to wonder how function overloads ever worked at all in my patch. On the other hand though, this optimization probably could be documented better, but I very well might have missed it even if it were. Hell, maybe it needs a greater redesign altogether, it seems strange to me to bundle overload information in with a construct for a specific expression. (Assuming that's whats happening of course, I still don't fully understand it.) It's not like this has rules unique to it for how overload resolution is decided, right? Initializing a param/variable of pointer to function type with an overloaded function resolves that with similar rules, I think? Maybe it is a little different now that I write it out loud. I wasn't going to finish my musings about that, but it made me realize that it might not actually be correct for address of explicit-object member functions to be wrapped by OFFSET_REF. I mean surely it's fine because based on what you've said static member functions are also wrapped by OFFSET_REF, so it's likely fully implemented, especially considering things worked before. But now that there are 2 different varieties of class members that the address of them can be taken, it might make sense to split things up a bit? Then again, why were static member functions ever handled the same way? Taking the address of other static members isn't handled in the same way here is it? I'm probably spending too much time thinking about it when I don't fully understand how it's all being done, I'll just go back to poking around trying to figure it all out. Then I'll worry about whether thing's should be done differently or not. Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> A BASELINK expresses the result of name lookup for a member function, > since we need to pass information about the name lookup context along to > after overload resolution. > > An OFFSET_REF (with PTRMEM_OK_P) is used to express that we saw the > ::f syntax, so we could build a pointer to member if it resolves to an > implicit-object member function. > > For an overload set containing only a single static member function, > build_offset_ref doesn't bother to build an OFFSET_REF, but returns the > BASELINK itself. > > I think we need the OFFSET_REF for an explicit-object member function > because it expresses that the code satisfies the requirement "If the > operand names an explicit object member function, the operand shall be a > qualified-id." > > It might simplify things to remove the optimization in build_offset_ref > so we get an OFFSET_REF even for a single static member function, and > add support for that to cp_build_addr_expr_1. > > Jason Ah okay I think that sheds a little bit of light on things, and here I was trying not to involve overloads to make it easier for me to understand things, it seems it ended up making me miss some things. At a glance it seems like all I need to do then is disable the PTRMEM_OK_P flag then. I will try that and see how it goes, provided I can find where it's all setup. (I think I'm almost to the bottom of it, but it's tough to unravel so I'm not sure.) I'm also now realizing it's probably about time I add more tests involving overloads, because I avoided that early on and I don't think I ever got around to adding any for that. Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> No, that wouldn't be appropriate for translation.
> None of non-member, static member and explicit object member are
> something that should be printed verbatim untranslated.
> "%s function %qD cannot have cv-qualifier", _("non-member")
> etc. is still inappropriate, some language might need to reorder
> the words in one case and not another one etc.
>
> What is ok if that %s (but in that case better %qs) is say some
> language keyword which shouldn't be translated.
>
> "%qs keyword not expected"
> with
> the arg being "inline", "constexpr", "consteval" for example would
> be fine.
>
> Jakub
Ah alright, I see what you're saying, I see what the difference is now.
It's a shame we can't have the translated string insert a %s and format
into that :^). Ah well, I guess this code is just doomed to look poor
then, what can you do.
This is pretty much why I've been afraid to touch anything with these
macros, if I don't understand exactly how it works anything slightly
weird solution I use might just not work.
Anyway, I think that clears up everything regarding that now, thanks.
Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> (Jakub) > There are different kinds of format strings in GCC, the most common > are the gcc-internal-format strings. If you call a function which > is expected to take such translatable format string (in particular > a function which takes a gmsgid named argument like error, error_at, > pedwarn, warning_at, ...) and pass a string literal to that function, > nothing needs to be marked in a special way, both gcc/po/exgettext > is able to collect such literals into gcc/po/gcc.pot for translations > and the function is supposed to use gettext etc. to translate it > - e.g. see diagnostic_set_info using (gmsgid) for that. > But, if there is e.g. a temporary pointer var which points to format > strings and only that is eventually passed to the diagnostic functions, > gcc/po/exgettext won't be able to collect such literals, which is where > the G() macro comes into play and one marks the string as > gcc-internal-format with it; the translation is still handled by the > diagnostic function at runtime. The N_() macro is similar but for c-format > strings instead. The _() macro both collects for translations if it is > used with string literal, and expands to gettext call to translate it at > runtime, which is something that should be avoided if something translates > it again. > (Jason) > The protocol is described in gcc/ABOUT-GCC-NLS. In general, "strings" > passed directly to a diagnostic function don't need any decoration, but > if they're assigned to a variable first, they need G_() so they're > recognized as diagnostic strings to be added to the translation table. I read this last night and decided to sleep on it hoping it would make more sense now. I was going to write about how I'm still confused but I think it just clicked for me. I somehow didn't make the connection that any kind of of expression (like a conditional) is going to require it's use. I guess I kept thinking "but they aren't being assigned to a variable" but I get it now, it's when the strings aren't passed DIRECTLY into the diagnostic function. > (Jakub) > And another i18n rule is that one shouldn't try to construct diagnostic > messages from parts of english sentences, it is fine to fill in with %s/%qs > etc. language keywords etc. but otherwise the format string should contain > the whole diagnostic line, so that translators can reorder the words etc. > (Jason) > The () macro is used for strings that are going to be passed to a %s, > but better to avoid doing that for strings that need translation. N() > is (rarely) used for strings that aren't diagnostic format strings, but > get passed to another function that passes them to _(). Okay so taking what you guys are saying here it sounds like it would be appropriate to refactor the code I was reluctant to refactor. The code (in grokfndecl) conditionally selects one of the two (now three with my changes) following strings despite them being mostly identical. "non-member function %qD cannot have cv-qualifier" "static member function %qD cannot have cv-qualifier" "explicit object member function %qD cannot have cv-qualifier" >From what I'm getting from what you two have said is that it would be reasonable to instead construct a string from it's two parts, just selecting the differing part dynamically. "%s function %qD cannot have cv-qualifier" Just to clarify, should I be marking the "non-member", "static member", "explicit object member" strings with the _ macro then? I'm just not sure since it seems like Jason is saying they shouldn't be since the string they are being formatted into is being translated afterwards. Also, I'm not sure what %qs is, should I be using that instead of %s for strings? On another topic, I have been trying to fix taking pointers to explicit object member functions today, as I ended up breaking that when I started setting static_function to false for them. Originally it just worked so I haven't touched any code related to this, but now that they aren't being treating like static member functions as much so a few things just broke. What I'm asking myself now is whether it would be appropriate to just opt-in to static member function behavior for this one, and I'm not sure that would be correct. So I started by checking what it did before I turned off the static_function flag. It's was being passed into cp_build_addr_expr_1 as a baselink node, while regular member functions are passed in as an offset_ref node. I then checked what the case is for static member function, and unsurprisingly those are also handled wrapped in a baselink node, but this raised some questions for me. I am now trying to figure out what exactly a baselink is, and why it's used for static member functions. My current best guess is that method_type nodes already hold the information that a baselink does, and that information is needed in general. If that is the case, it might just be correct to just do the same thing for explicit object member functions, but I wonder if there is more
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> > I will try to get something done today, but I was struggling with
> > writing some of the tests, there's also a lot more of them now. I also
> > wrote a bunch of musings in comments that I would like feedback on.
> >
> > My most concrete question is, how exactly should I be testing a
> > pedwarn, I want to test that I get the correct warning and error with
> > the separate flags, do I have to create two separate tests for each one?
>
>
> Yes. I tend to use letter suffixes for tests that vary only in flags
> (and expected results), e.g. feature1a.C, feature1b.C.
Will do.
> Instead of OPT_Wpedantic, this should be controlled by
> -Wc++23-extensions (OPT_Wc__23_extensions)
Yeah, I'll do this.
> If you wanted, you could add a more specific warning option for this
> (e.g. -Wc++23-explicit-this) which is also affected by
> -Wc++23-extensions, but I would lean toward just using the existing
> flag. Up to you.
I brought it up in irc and there was some pushback to my point of view
on it, so I'll just stick with OPT_Wc__23_extensions for now. I do
think a more sophisticated interface would be beneficial but I will
bring discussion around that up again in the future.
I've seen plenty of these G_ or _ macros on strings around like in
grokfndecl for these errors.
G_("static member function %qD cannot have cv-qualifier")
G_("non-member function %qD cannot have cv-qualifier")
G_("static member function %qD cannot have ref-qualifier")
G_("non-member function %qD cannot have ref-qualifier")
I have been able to figure out it relates to translation, but not
exactly what the protocol around them is. I think in my original patch
I had refactored this code a bunch, I figured adding a 3rd case to it
justifies a refactor. I think I forgot to add those changes to the
original patch, either that or I undid it or moved it somewhere else.
Anyway, the point is, coming back to it now to re-add those diagnostics
I realized I probably shouldn't have changed those strings.
I also have been wondering whether I should be putting macros on any
strings I add, it seemed like there might have been a macro for text
that needs translation. Is this something I should be doing?
Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> Any progress on this, or do I need to coax the process along? :) Yeah, I've been working on it since the copyright assignment process has finished, originally I was going to note that on my next update which I had hoped to finish today or tomorrow. Well, in truth I was hoping to send one the same day that copyright assignment finished, but I found a nasty bug so I spent all day adding test cases for all the relevant overloadable operators. Currently, it crashes when calling a subscript operator declared with an explicit object parameter in a dependent context. I haven't looked into the fix yet, but I plan to. Also, before I forget, what is the process for confirming my copyright assignment on your end? Do you just need to check in with the FSF to see if it went through? Let me know if there's anything you need from me regarding that. Aside from the bug that's currently present in the first patch, I only have like 1 or 2 little things I want to change about it. I will make those few changes to patch 1, finish patch 2 (diagnostics) which will also include test cases for the new bug I found. After I am done that I plan on adding the things that are missing, because I suspect that looking into that will get me close to finding the fix for the crash. > Hmm, is it? I see that clang thinks so, but I don't know where they get > that idea from. The grammar certainly allows it: > > > attribute-specifier-seqopt decl-specifier-seq declarator = > > initializer-clause > > > and I don't see anything else that prohibits it. You would be right for P0847R7, but CWG DR 2653 changed that. You can find the updated grammar in dcl.fct section 3 (subsection? I'm not really sure to be honest.) I've also included a copy of the grammar here for your convenience. https://eel.is/c++draft/dcl.fct#nt:parameter-declaration parameter-declaration: attribute-specifier-seqopt thisopt decl-specifier-seq declarator attribute-specifier-seqopt decl-specifier-seq declarator = initializer-clause attribute-specifier-seqopt thisopt decl-specifier-seq abstract-declaratoropt attribute-specifier-seqopt decl-specifier-seq abstract-declaratoropt = initializer-clause > I was thinking to set a TREE_LANG_FLAG on the TREE_LIST node. I did figure this is originally what you meant, and I can still change it to go this route since I'm sure it's likely just as good. But I do recall something I didn't like in the implementation that nudged me towards using the purpose member instead. Either way, not a big deal. I think I just liked not having to mess with a linked list as I am not used to them as a data structure, it might have been that simple. :^) I will try to get something done today, but I was struggling with writing some of the tests, there's also a lot more of them now. I also wrote a bunch of musings in comments that I would like feedback on. My most concrete question is, how exactly should I be testing a pedwarn, I want to test that I get the correct warning and error with the separate flags, do I have to create two separate tests for each one? I'm just going to include the little wall I wrote in decl.cc regarding pedwarn, just in case I can't get this done tonight so I can get some feedback regarding it. On the other hand, it might just not be very relevant to this patch in particular as I kind of noted, but maybe there's some way to do what I was musing about that I've overlooked. It does end up a bit ranty I guess, hopefully that doesn't make it confusing. ``` /* I believe we should make a switch for this feature specifically, I recall seeing discussion regarding enabling newer language features when set to older standards. I would advocate for a specific flag for each specific feature. Maybe they should all be under an override flag? -foverride-dialect=xobj,ifconstexpr (?) I dont think it makes sense to give each feature override it's own flag. I don't recall what the consensus was around this discussion either though. For the time being it's controlled by pedantic. I am concerned that tying this to pedantic going forward that one might want to enable -pedantic-errors while also enabling select features from newer dialects. It didn't look like this use case is supported to me. I suppose this will require redesign work to support, so for the purposes of this patch, emitting a pedwarn seems correct. I just don't like that it can't be suppressed on an individual basis. */ if (xobj_parm && cxx_dialect < cxx23) pedwarn(DECL_SOURCE_LOCATION (xobj_parm), OPT_Wpedantic, ""); ``` That's all for now, I will try, (but I am very much not promising,) to have an update by the end of today (6-8 hours for me.) If I manage to get that out, I will (finally) start moving forward on implementing the missing and broken features, and the aforementioned bug. Alex
Re: [PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Not to worry, I'm currently going through that process with the FSF, it was confirmed that a pseudonym should be just fine. I don't know how long the process takes but my goal is to get this in for GCC14, and surely this won't take more than a month. One can only hope anyway. On 2023-09-27 04:43 p.m., Hans-Peter Nilsson wrote: >> Date: Tue, 26 Sep 2023 01:56:55 + >> From: waffl3x > >> Signed-off-by: waffl3x > > I think I've read that you have to put your actual name in > the DCO; using an alias (presumably) as above would be > wrong. > > Ah, it's on https://gcc.gnu.org/dco.html - the *second* DCO > link; under "Signed-off-by", on > https://gcc.gnu.org/contribute.html! "sorry, no pseudonyms > or anonymous contributions". > > (Also, from Some Source I Don't Remember: using an alias if > you have FSF papers in place is ok; you can use a pseudonym > if FSF can match it to papers on file that have your actual > name or something to that effect.) > > brgds, H-P
[PATCH v3 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> Yes, but I'll warn you that grokdeclarator has resisted refactoring for
> a long time...
That will certainly be what I work on after this is squared off then,
I've been up and down grokdeclarator so I'm confident I'll be able to
do it.
As for the patch, I sure took my sweet time with it, but here it is. I
hope to work on the diagnostics patch tomorrow, but as you've probably
figured out it's best not to take my word on timeframes :^).
On the plus side, I took my time to figure out how to best to pass down
information about whether a param is an xobj param. My initial
impression on what you were suggesting was to push another node on the
front of the list, but I stared at it for a few hours and didn't think
it would work out. However, eventually I realized that the purpose
member if free for xobj params as it is illegal for them to have
default arguments. So I ended up passing it over the TREE_LIST after
all, maybe this is what you meant in the first place anyway too.
I am pretty confident that this version is all good, with only a few
possible issues.
An update on my copyright assignment, I sent an e-mail and haven't
gotten a response yet. From what I saw, I am confident that it's my
preferred option going forward though. Hopefully they get back to me
soon.
Also, just a quick update on my copyright assignment, I have sent an
e-mail to the FSF and haven't gotten a response yet. From what I was
reading, I am confident that it's my preferred option going forward
though. Hopefully they get back to me soon.
Bootstrapped and regtested on x86_64-pc-linux-gnu.
From bbfbcc72e8c0868559284352c71731394c98441e Mon Sep 17 00:00:00 2001
From: waffl3x
Date: Mon, 25 Sep 2023 16:59:10 -0600
Subject: [PATCH] c++: Initial support for C++23 P0847R7 (Deducing This)
[PR102609]
This patch implements initial support for P0847R7, without additions to
diagnostics. Almost everything should work correctly, barring a few
limitations which are listed below. I attempted to minimize changes to the
existing code, treating explicit object member functions as static functions,
while flagging them to give them extra powers seemed to be the best way of
achieving this. For this patch, the flag is only utilized in call.cc for
resolving overloads and making the actual function call.
Internally, the difference between a static member function and an implicit
object member function appears to be whether the type node of the decl is a
FUNCTION_TYPE or a METHOD_TYPE. So to get the desired behavior, it seems to be
sufficient to simply prevent conversion from FUNC_TYPE to METHOD_TYPE in
grokdeclarator when the first parameter is an explicit object parameter. To
achieve this, explicit object parameters are flagged as such through each the
TREE_LIST's purpose member in declarator->u.function.parameters. Typically the
purpose member is used for default arguments, as those are not allowed for
explicit object parameters, we are able to repurpose purpose for our purposes.
The value used as a flag is the "this_identifier" global tree, as it seemed to
be the most fitting of the current global trees. Even though it is obviously
illegal for any parameter except the first to be an explicit object parameter,
each parameter parsed as an explicit object parameter will be flagged in this
manner. This will be used for diagnostics in the following patch. When an
explicit object parameter is encountered in grokdeclarator, the purpose member
is nulled before the list is passed elsewhere to maintain compatibility with
any code that assumes that a non-null purpose member indicates a default
argument. This patch only checks for and nulls the first parameter however.
As for the previously mentioned limitations, lambdas do not work correctly yet,
but I suspect that a few tweaks are all it will take to have them fully
functional. User defined conversion functions are not called when an explicit
object member function with an explicit object parameter of an unrelated type
is called. The following case does not behave correctly because of this.
struct S {
operator size_t() {
return 42;
}
size_t f(this size_t n) {
return n;
}
};
int main()
{
S s{};
size_t a = s.f();
}
Currently, it appears that the object argument is simply reinterpreted as
a size_t instead of properly calling the user defined conversion function.
The validity of such a conversion is still considered however, if there is no
way to convert S to a size_t an appropriate compile error will be emitted.
I have an idea of what changes need to be made to fix this, but I did not
persue this for the initial implementation patch.
This bug can be observed in the explicit-object-param4.C test case, while
explicit-object-param3.C demonstrates the non functioning lambdas.
PR c++/102609
gcc/cp/ChangeLog:
PR c++/102609
Initial support for C++23 P0847R7 - Deducing this.
* call.cc (add_candidates): Check if fn is an xobj member function.
(build_over_
Re: [PATCH v2 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> This seems like a reasonable place for it since 'this' is supposed to > precede the decl-specifiers, and since we are parsing initial attributes > here rather than in the caller. You will want to give an error if > found_decl_spec is set. And elsewhere complain about 'this' on > parameters after the first (in cp_parser_parameter_declaration_list?), > or in a non-member/lambda (in grokdeclarator?). Bringing this back up, I recalled another detail regarding this. I'm pretty sure that found_decl_spec can be false when parsing the second or latter decl-specifier. I tested it quickly and I believe I am correct. I raise this as my diagnostics patch introduces another variable to track whether we are on the first decl-specifier, given the results of my quick test, I believe that was the correct choice. This kinda unclear machinery is what makes me really want to refactor this code, but I've resisted as it would be inappropriate to try to do so while implementing a feature. Once I am finished implementing `deducing this` would you be open to me refactoring grokdeclarator and it's various auxiliary functions? As for where the complaining happens, I believe I implemented this particular error in cp_parser_decl_specifier_seq, I don't plan to be stubborn on any of the diagnostic code though as I'm pretty unhappy with how it got scattered about. I intend to get more input on that after I finish v2 of the diagnostic patch though. > That's a good point, but the flag you chose seems even more general purpose. Yeah, I had to just settle on it because I was bikeshedding it for a couple hours despite being very unhappy with it. > A better option might be, instead of putting this flag on the PARM_DECL, > to put it on the short-lived TREE_LIST which is only used for > communication between cp_parser_parameter_declaration_list and > grokparms, and have grokdeclarator grab it from > declarator->u.function.parameters? That does sound ideal! I will look into doing it this way. > Generally the flags that aren't specifically specified to be > language-specific are reserved for language-independent uses; even if > only one front-end actually uses the feature, it should be for > communication to language-independent code rather than communication > within the particular front-end. Ah okay, that makes perfect sense to me, understood. > The patch modified tree-core.h to > refer to a macro in cp-tree.h. Yeah, I wasn't sure about doing that, I will refrain from that in the future, (along with removing it from v3, but the other change you suggested should eliminate the referred to macro anyway.) > > Yeah, I separated all the diagnostics out into the second patch. This > > patch was meant to include the bare minimum of what was necessary to > > get the feature functional. As for the diagnostics patch, I'm not happy > > with how scattered about the code base it is, but you'll be able to > > judge for yourself when I resubmit that patch, hopefully later today. > > So not to worry, I didn't neglect diagnostics, it's just in a follow > > up. The v1 of it was submitted on August 31st if you want to find it, > > but I wouldn't recommend it. I misunderstood how some things were to be > > formatted so it's probably best you just wait for me to finish a v2 of > > it. > > > Ah, oops, I assumed that v2 completely replaced v1. I had intended to complete v2 of it quite some time ago, I've just been busy. Today as well I got sidetracked with some job hunting, but I plan on finishing v3 of the initial support patch (the one related to this thread) tonight at the very least. I can't commit to diagnostics v2 tonight, but if it happens it happens. :) I might even have to leave out communicating that a PARM_DECL is an xobj parm cp_parser_parameter_declaration_list if I have too hard a time figuring out how to work it in, if that is the case then I will make that change in a v4. Alex
[PATCH v2 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
> Thank you, this is great!
Thanks!
> One legal hurdle to start with: our DCO policy
> (https://gcc.gnu.org/dco.html) requires real names in the sign-off, not
> pseudonyms. If you would prefer to contribute under this pseudonym, I
> encourage you to file a copyright assignment with the FSF, who are set
> up to handle that.
I will get on that right away.
> > +/* These need to moved to somewhere appropriate. */
>
>
> This isn't a bad spot for these macros, but you could also move them
> down lower, maybe near DECL_THIS_STATIC and DECL_ARRAY_PARAMETER_P for
> some thematic connection.
Sounds good, I will move them down.
> > +/* The flag is a member of base, but the value is meaningless for other
> > + decl types so checking is still justified I imagine. */
>
>
> Absolutely, we often reuse bits for other purposes if they're disjoint
> from the use they were added for.
Would it be more appropriate to give it a general name in base instead
then? If so, I can also change that.
> > +/* Not a lang_decl field, but still specific to c++. */
> > +#define DECL_PARM_XOBJ_FLAG(NODE) \
> > + (PARM_DECL_CHECK (NODE)->decl_common.decl_flag_3)
>
>
> Better to use a DECL_LANG_FLAG than claim one of the
> language-independent flags for C++.
>
> There's a list at the top of cp-tree.h of the uses of LANG_FLAG on
> various kinds of tree node. DECL_LANG_FLAG_4 seems free on PARM_DECL.
Okay, I will switch to that instead, I didn't like using such a general
purpose flag for what is only relevant until the FUNC_DECL is created
and then never again.
If you don't mind answering right now, what are the consequences of
claiming language-independent flags for C++? Or to phrase it
differently, why would this be claiming it for C++? My guess was that
those flags could be used by any front ends and there wouldn't be any
conflicts, as you can't really have crossover between two front ends at
the same time. Or is that the thing, that kind of cross-over is
actually viable and claiming a language independent flag inhibits that
possibility? Like I eluded to, this is kinda off topic from the patch
so feel free to defer the answer to someone else but I just want to
clear up my understanding for the future.
> > + /* Only used for skipping over build_memfn_type, grokfndecl handles
> > + copying the flag to the correct field for a func_decl.
> > + There must be a better way to do this, but it isn't obvious how. */
> > + bool is_xobj_member_function = false;
> > + auto get_xobj_parm = [](tree parm_list)
>
>
> I guess you could add a flag to the declarator, but this is fine too.
> Though I'd move this lambda down into the cdk_function case or out to a
> separate function.
Okay, I will move the lambda.
> > case cdk_function:
> > {
> > + tree xobj_parm
> > + = get_xobj_parm (declarator->u.function.parameters);
> > + is_xobj_member_function = xobj_parm;
>
>
> I'd also move these down a few lines after the setting of 'raises'.
Will do.
Also, I forgot to mention it anywhere, the diagnostic patch utilizes
xobj_parm which is why it's a separate variable.
> > + /* Set the xobj flag for this parm, unfortunately
> > + I don't think there is a better way to do this. */
> > + DECL_PARM_XOBJ_FLAG (decl)
> > + = decl_spec_seq_has_spec_p (declspecs, ds_this);
>
>
> This seems like a fine way to handle this.
Okay good, I had my doubt's there.
> > + /* Special case for xobj parm, doesn't really belong up here
> > + (it applies to parm decls and those are mostly handled below
> > + the following specifiers) but I intend to refactor this function
> > + so I'm not worrying about it too much.
> > + The error diagnostics might be better elsewhere though. */
>
>
> This seems like a reasonable place for it since 'this' is supposed to
> precede the decl-specifiers, and since we are parsing initial attributes
> here rather than in the caller. You will want to give an error if
> found_decl_spec is set. And elsewhere complain about 'this' on
> parameters after the first (in cp_parser_parameter_declaration_list?),
> or in a non-member/lambda (in grokdeclarator?).
>
> Jason
Yeah, I separated all the diagnostics out into the second patch. This
patch was meant to include the bare minimum of what was necessary to
get the feature functional. As for the diagnostics patch, I'm not happy
with how scattered about the code base it is, but you'll be able to
judge for yourself when I resubmit that patch, hopefully later today.
So not to worry, I didn't neglect diagnostics, it's just in a follow
up. The v1 of it was submitted on August 31st if you want to find it,
but I wouldn't recommend it. I misunderstood how some things were to be
formatted so it's probably best you just wait for me to finish a v2 of
it.
One last thing, I assume I should clean up the comments and replace
them with more typical ones right? I'm going to go forward with that
assumption in v3, I just want to mention it in advanced just in case I
have the wrong idea.
I will get started on v3
[PATCH v2 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions.
Hopefully I fixed all the issues. I also took the opportunity to remove the
small mistake present in v1, so that is no longer a concern.
Thanks again for all the patience.
-AlexFrom 0db52146880faf20e7a7b786dad47c686a5f26d6 Mon Sep 17 00:00:00 2001
From: Waffl3x
Date: Mon, 11 Sep 2023 04:21:10 -0600
Subject: [PATCH] c++: Initial support for C++23 P0847R7 (Deducing This)
[PR102609]
This patch implements initial support for P0847R7, without additions to
diagnostics. Almost everything should work correctly, barring a few
limitations which are listed below. I attempted to minimize changes to the
existing code, treating explicit object member functions as static functions,
while flagging them to give them extra powers seemed to be the best way of
achieving this. For this patch, the flag is only utilized in call.cc for
resolving overloads and making the actual function call. To achieve this,
conversion of a FUNC_TYPE to a METHOD_TYPE is suppressed when the first
parameter of the FUNC_TYPE is an explicit object parameter, this appears to be
sufficient. I opted to create a new variable to achieve this instead of
using "staticp" to avoid any possible confusion.
As for the previously mentioned limitations, lambdas do not work correctly yet,
but I suspect that a few tweaks are all it will take to have them fully
functional. User defined conversion functions are not called when an explicit
object member function with an explicit object parameter of an unrelated type
is called. The following case does not behave correctly because of this.
struct S {
operator size_t() {
return 42;
}
size_t f(this size_t n) {
return n;
}
};
int main()
{
S s{};
size_t a = s.f();
}
Currently, it appears that the object argument is simply reinterpreted as
a size_t instead of properly calling the user defined conversion function.
The validity of such a conversion is still considered however, if there is no
way to convert S to a size_t an appropriate compile error will be emitted.
I have an idea of what changes need to be made to fix this, but I did not
persue this for the initial implementation patch.
This bug can be observed in the explicit-object-param4.C test case, while
explicit-object-param3.C demonstrates the non functioning lambdas.
PR c++/102609
gcc/cp/ChangeLog:
PR c++/102609
Initial support for C++23 P0847R7 - Deducing this.
* call.cc (add_candidates): Check if fn is an xobj member function.
(build_over_call): Ditto.
* cp-tree.h (struct lang_decl_base::xobj_flag): New data member.
(DECL_FUNC_XOBJ_FLAG): Define.
(DECL_PARM_XOBJ_FLAG): Define.
(DECL_IS_XOBJ_MEMBER_FUNC): Define.
(enum cp_decl_spec): Add ds_this.
* decl.cc (grokfndecl): Set xobj_flag if first param is an xobj param.
(grokdeclarator): For xobj member functions, Don't change type to
METHOD_TYPE, leave it as FUNC_TYPE. For PARM decl_context, set
decl_flag_3 if param is an xobj param.
* parser.cc (cp_parser_decl_specifier_seq): Handle this specifier.
(set_and_check_decl_spec_loc): Add "this".
gcc/ChangeLog:
PR c++/102609
Initial support for C++23 P0847R7 - Deducing this.
* tree-core.h (struct tree_decl_common): Comment use of decl_flag_3.
gcc/testsuite/ChangeLog:
PR c++/102609
Initial support for C++23 P0847R7 - Deducing this.
* g++.dg/cpp23/explicit-object-param1.C: New test.
* g++.dg/cpp23/explicit-object-param2.C: New test.
* g++.dg/cpp23/explicit-object-param3.C: New test.
* g++.dg/cpp23/explicit-object-param4.C: New test.
Signed-off-by: Waffl3x
---
gcc/cp/call.cc| 6 +-
gcc/cp/cp-tree.h | 21 +++-
gcc/cp/decl.cc| 24
gcc/cp/parser.cc | 15 ++-
.../g++.dg/cpp23/explicit-object-param1.C | 114 ++
.../g++.dg/cpp23/explicit-object-param2.C | 28 +
.../g++.dg/cpp23/explicit-object-param3.C | 15 +++
.../g++.dg/cpp23/explicit-object-param4.C | 33 +
gcc/tree-core.h | 3 +-
9 files changed, 254 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param1.C
create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param2.C
create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param3.C
create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param4.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 399345307ea..37690fdae25 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -6547,7 +6547,8 @@ add_candidates (tree fns, tree first_arg, const vec *args,
tree fn_first_arg = NULL_TREE;
const vec *fn_args = args;
- if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn))
+ if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
+ || DECL_IS_XOBJ_MEMBER_FUNC (fn))
{
/* Figure out where the object arg comes from. If this
function is a non-static memb
Re: [PATCH 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Hey Jakub, thanks for the response and criticism, as soon as I am back at a computer I will address the issues you raised, I have a few questions though. I apologize in advanced for any errors in formatting this message, I'm writing it from a hotel room on a phone so errors are inevitable, but I'll try my best. >More importantly, should describe >what changed and not why I was under the impression that if someone wants to see the what, they can just check the diff. I don't understand what should be written if I'm just to say what, wouldn't I just be repeating what is already said in the code itself? >I think usually we don't >differentiate >in testcase names whether Yeah, for sure, but I felt like there is value in differentiating them and that it would be harmless to do so. If you feel like it's more important that convention is followed here I won't object, but I think this should be considered. >Isn't explicit-object-param too long >though? >explicit-this or deducing-this might >be shorter... I agree, but I felt like I should stick to the wording of the standard for it, but I don't feel strongly about that justification, so I wouldn't object to changing it. Truthfully I flip flopped many times around the names, I'll defer to whatever is decided by the maintainers on that without complaint. >> +} >> \ No newline at end of file >Please avoid these Yeah, I noticed it last minute and wasn't sure how big a deal it was. I will make sure to fix it along with everything else you noted. >Usually runtime tests don't try to >print something Yeah, I didn't like printing, but I'm not sure I like aborting either. I value granularity in testing, if one part of the test passes but another fails, you would want to know that. If you abort before the second you lose that granularity. Once again, I'll defer to the maintainers for this, but I think my points are valid here. >This raises an important question whether we as an extension >should support deducing this even in older standards or not. >I admit I haven't studied the paper enough to figure that out. I'm glad you think so, I fully agree. I had planned to raise that once the initial patch made it in. I don't believe there is anything in the paper in particular that breaks previous standards. I can imagine there being problems if older projects tried to convert everything to use it as there are alignment differences (at least in the current patch version) between explicit object member functions and implicit object member functions. This is mostly a side effect of treating them as static functions most of the time, but I wasn't sure what would be ideal, nor how to change it. I decided that the difference was likely mainly due to virtual functions (if you know more about this, please be sure to correct me), and as virtual is not (currently) allowed, I just left it as it is. In short, I agree, and furthermore I think the syntax should be allowed with virtual just so style can be maintained. That would have greater implications than what you mentioned though and would need some extra hacks to make work, instead of just allowing what should -just- work. Thanks again for the input, I will get on it asap. Unfortunately that will be in a while, but I am determined to get this feature into GCC14 so one way or another I will make it happen. -Alex
[PATCH 2/2] c++: Extended diagnostics for P0847R7 (Deducing This) [PR102609]
Tested and Bootstrapped and tested on x86_64-linux with no regressions. There's a few test cases that are not properly diagnosed yet, but everything that is known to fail is marked as xfail. When I tested the new tests I got 390 expected passes and 64 expected failures. Alright, I have a flight to catch, hopefully my patch is at least satisfactory. There's a few thing's I'm not totally happy with so I will be happy to address any criticism as soon as I am able. Thanks again to everyone who helped me prepare this, I probably would have given up otherwise. From d82a34432364b391abde44a23ceacb3c398a519d Mon Sep 17 00:00:00 2001 From: Waffl3x Date: Thu, 31 Aug 2023 02:13:52 -0400 Subject: [PATCH 2/2] P0847R7 (deducing this) Extended diagnostics gcc/cp/ChangeLog: * cp-tree.h (TFF_XOBJ_FUNC): new flag to identify that an explicit object member function's parameters are being printed * decl.cc (grokdeclarator): diagnose type declarations using 'this', diagnose function declarations using this (grokparms): diagnose an explicit object parameter with a default argument * error.cc (dump_function_decl): prevent explicit object member functions from being pretty printed with 'static', communicate to dump_parameters that the current function is an explicit object member function (dump_parameters): pretty print the explicit object parameter with a leading 'this' (function_category): support reporting that the current context is within an explicit object member function * parser.cc (cp_parser_decl_specifier_seq): diagnose uses of 'this' when it is not the first specifier of a decl-specifier-seq (cp_parser_parameter_declaration_list): diagnose when an explicit object parameter is not the first parameter * semantics.cc (finish_this_expr): diagnose when 'this' is used in the body of an explicit object member function gcc/testsuite/ChangeLog: * g++.dg/cpp23/explicit-object-param-ill-formed1.C: New test. * g++.dg/cpp23/explicit-object-param-ill-formed2.C: New test. * g++.dg/cpp23/explicit-object-param-ill-formed3.C: New test. * g++.dg/cpp23/explicit-object-param-ill-formed4.C: New test. * g++.dg/cpp23/explicit-object-param-ill-formed5.C: New test. * g++.dg/cpp23/explicit-object-param-ill-formed6.C: New test. * g++.dg/cpp23/explicit-object-param-no-cxx23.C: New test. Signed-off-by: Waffl3x --- gcc/cp/cp-tree.h | 5 +- gcc/cp/decl.cc| 71 + gcc/cp/error.cc | 12 +- gcc/cp/parser.cc | 62 gcc/cp/semantics.cc | 24 ++- .../cpp23/explicit-object-param-ill-formed1.C | 141 ++ .../cpp23/explicit-object-param-ill-formed2.C | 25 .../cpp23/explicit-object-param-ill-formed3.C | 102 + .../cpp23/explicit-object-param-ill-formed4.C | 42 ++ .../cpp23/explicit-object-param-ill-formed5.C | 9 ++ .../cpp23/explicit-object-param-ill-formed6.C | 22 +++ .../cpp23/explicit-object-param-no-cxx23.C| 7 + 12 files changed, 517 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param-ill-formed1.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param-ill-formed2.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param-ill-formed3.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param-ill-formed4.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param-ill-formed5.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param-ill-formed6.C create mode 100644 gcc/testsuite/g++.dg/cpp23/explicit-object-param-no-cxx23.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 3aca23da105..325d0fb3ca0 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6111,7 +6111,9 @@ enum auto_deduction_context identical to their defaults. TFF_NO_TEMPLATE_BINDINGS: do not print information about the template arguments for a function template specialization. - TFF_POINTER: we are printing a pointer type. */ + TFF_POINTER: we are printing a pointer type. + TFF_XOBJ_FUNC: we are printing an explicit object member function's + parameters */ #define TFF_PLAIN_IDENTIFIER (0) #define TFF_SCOPE(1) @@ -6129,6 +6131,7 @@ enum auto_deduction_context #define TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS (1 << 12) #define TFF_NO_TEMPLATE_BINDINGS (1 << 13) #define TFF_POINTER (1 << 14) +#define TFF_XOBJ_FUNC(1 << 15) /* These constants can be used as bit flags to control strip_typedefs. diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index a6d0cfb0ecc..da9235ced30 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -13080,6 +13080,67 @@ grokdeclarator (const cp_declarator *declarator, tree xobj_parm = get_xobj_parm (declarator->u.function.parameters); is_xobj_member_function = xobj_parm; + if (!xobj_pa
[PATCH 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609]
Bootstrapped and tested on x86_64-linux with no regressions. I would like to quickly thank everyone who helped me for their patience as I learned the ropes of the codebase and toolchain. It is much appretiated and this would have been much much more difficult without the support. This patch handles the new explicit object member functions by bypassing special behavior of implicit object member functions, but opting back into the special behavior during a function call through member access. This is mainly accomplished by bypassing becoming a METHOD_TYPE and remaining as a FUNCTION_TYPE. Normally, this would be treated as a static member function, but a new explicit object member function flag is added to lang_decl_base and is set for the declaration of explicit object member functions. This sets it apart from static member functions when it is relevant, and is the criteria used to opt-in to passing the implicit object argument during a member function call. The benefit of this design is less code needs to be modified to support the new feature, as most of the semantics of explicit object member functions matches those of static member functions. There is very little left to add, and hopefully there are few bugs in the implementation despite the minimal changes. It is possible there are hidden problems with passing the implicit object argument, but none of the tests I made exhibit such a thing EXCEPT for in the pathological case as I describe below. Upon reflection, a by value explicit object parameter might be broken as well, I can't recall if there's a good test for that case. Lambdas do not work yet, but you can work around it by marking it as mutable so I suspect it could be supported with minimal changes, I just ran out of time. The other thing that does not work is the pathological case with an explicit object parameter of an unrelated type and relying on a user-defined conversion operator to cast to said type in a call to that function. You can observe the failing test for that case in explicit-object-param-valid4.C, the result is somewhat interesting, but is also why I mention that there might be hidden problems here. I selectively excluded all the diagnostics from this patch, it's possible I made a mistake and the patch will be non-functional without the addition of the diagnostics patch. If that ends up being the case, please apply the following patch that includes the diagnostics and tests and judge the functionality from that. I believe that even if I mess up this patch, there should still be value in splitting up the changes into the two patches as it should make the changes to the behavior of the compiler much more clear. With that said, I believe I didn't make any mistakes while seperating the two patches, hopefully that is the case. I left in a FIXME (in call.cc) as I noticed last minute that I made a mistake, it should be benign and removing it appears to not break anything, but I don't have time to do another bootstrap at the moment. My priority is to get eyes on the changes I've made and recieve feedback. The patch including the diagnostics will follow shortly, assuming I don't run out of time and need to rush to catch my flight :). PS: Are there any circumstances where TREE_CODE is FUNCTION_DECL but the lang_specific member is null? I have a null check for that case in DECL_IS_XOBJ_MEMBER_FUNC but I question if it's necessary. From e485a79ec5656e72ba46053618843c3d69331eab Mon Sep 17 00:00:00 2001 From: Waffl3x Date: Thu, 31 Aug 2023 01:05:25 -0400 Subject: [PATCH] P0847R7 (deducing this) Initial support Most things should be functional, lambdas need a little more work though. Limitations: Missing support for lambdas, and user defined conversion functions when passing the implicit object argument does not work properly. See explicit-object-param-valid4.C for an example of the current (errent) behavior. There is a slight mistake in call.cc, it should be benign. gcc/cp/ChangeLog: * call.cc (add_function_candidate): (Hopefully) benign mistake (add_candidates): Treat explicit object member functions as member functions when considering candidates (build_over_call): Enable passing an implicit object argument when calling an explicit object member function * cp-tree.h (struct lang_decl_base): Added member xobj_flag for differentiating explicit object member functions from static member functions (DECL_FUNC_XOBJ_FLAG): New, checked access for xobj_flag (DECL_PARM_XOBJ_FLAG): New, access decl_flag_3 (DECL_IS_XOBJ_MEMBER_FUNC): New, safely check if a node is an explicit object member function (enum cp_decl_spec): Support parsing 'this' as a decl spec, change is mirrored in parser.cc:set_and_check_decl_spec_loc * decl.cc (grokfndecl): Sets the xobj flag for the FUNCTION_DECL if the first parameter is an explicit object parameter (grokdeclarator): Sets the xobj flag for PARM_DECL if 'this' spec is present in declspecs, bypasses conversion from
