From: Jason Merrill <jason@redhat.com>
To: waffl3x <waffl3x@protonmail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH v8 1/4] c++: P0847R7 (deducing this) - prerequisite changes. [PR102609]
Date: Tue, 9 Jan 2024 16:56:54 -0500 [thread overview]
Message-ID: <bdec7223-2d0c-41a6-8283-2d9ba52f0a60@redhat.com> (raw)
In-Reply-To: <2270f47c-f117-46a7-863e-f4cbfe03925b@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]
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:
[-- Attachment #2: 0001-c-explicit-object-cleanups.patch --]
[-- Type: text/x-patch, Size: 7356 bytes --]
From 5a6d3b1737843aa64d83ffc5d639fa0afa5d8318 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Tue, 9 Jan 2024 16:00:52 -0500
Subject: [PATCH 1/2] c++: explicit object cleanups
To: gcc-patches@gcc.gnu.org
The FIXME in xobj_iobj_parameters_correspond was due to expecting
TYPE_MAIN_VARIANT to be the same for all equivalent types, which is not the
case. And I adjusted some comments that I disagree with; the iobj parameter
adjustment only applies to overload resolution, we can handle that in
cand_parms_match (and I have WIP for that).
gcc/cp/ChangeLog:
* call.cc (build_over_call): Refactor handle_arg lambda.
* class.cc (xobj_iobj_parameters_correspond): Fix FIXME.
* method.cc (defaulted_late_check): Adjust comments.
---
gcc/cp/call.cc | 24 ++++++++++++------------
gcc/cp/class.cc | 40 ++++++++++++----------------------------
gcc/cp/method.cc | 7 +------
3 files changed, 25 insertions(+), 46 deletions(-)
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index dca8e5090e2..7d3d67600c8 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -10187,11 +10187,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
parm = TREE_CHAIN (parm);
}
- auto handle_arg = [fn, flags, complain](tree type,
- tree arg,
- int const param_index,
- conversion *conv,
- bool const conversion_warning)
+ auto handle_arg = [fn, flags](tree type,
+ tree arg,
+ int const param_index,
+ conversion *conv,
+ tsubst_flags_t const arg_complain)
{
/* Set user_conv_p on the argument conversions, so rvalue/base handling
knows not to allow any more UDCs. This needs to happen after we
@@ -10199,9 +10199,6 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
if (flags & LOOKUP_NO_CONVERSION)
conv->user_conv_p = true;
- tsubst_flags_t const arg_complain
- = conversion_warning ? complain : complain & ~tf_warning;
-
if (arg_complain & tf_warning)
maybe_warn_pessimizing_move (arg, type, /*return_p=*/false);
@@ -10214,13 +10211,12 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
if (DECL_XOBJ_MEMBER_FUNCTION_P (fn))
{
gcc_assert (cand->num_convs > 0);
- static constexpr bool conversion_warning = true;
tree object_arg = consume_object_arg ();
val = handle_arg (TREE_VALUE (parm),
object_arg,
param_index++,
convs[conv_index++],
- conversion_warning);
+ complain);
if (val == error_mark_node)
return error_mark_node;
@@ -10260,11 +10256,14 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
&& cand->template_decl
&& !cand->explicit_targs);
+ tsubst_flags_t const arg_complain
+ = conversion_warning ? complain : complain & ~tf_warning;
+
val = handle_arg (TREE_VALUE (parm),
current_arg,
param_index,
convs[conv_index],
- conversion_warning);
+ arg_complain);
if (val == error_mark_node)
return error_mark_node;
@@ -10273,7 +10272,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
}
/* Default arguments */
- for (; parm && parm != void_list_node; parm = TREE_CHAIN (parm), param_index++)
+ for (; parm && parm != void_list_node;
+ parm = TREE_CHAIN (parm), param_index++)
{
if (TREE_VALUE (parm) == error_mark_node)
return error_mark_node;
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index f3cfa9f0f23..e5e609badf3 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -1020,9 +1020,12 @@ modify_vtable_entry (tree t,
\f
/* Check if the object parameters of an xobj and iobj member function
- correspond. This function assumes that the iobj parameter has been correctly
- adjusted when the function is introduced by a using declaration per
- [over.match.funcs.general.4]. */
+ correspond. This function assumes that the iobj parameter has been
+ correctly adjusted when the function is introduced by a using declaration
+ per [over.match.funcs.general.4].
+
+ ??? But it isn't, that's only considered at overload resolution time.
+ cand_parms_match will probably need to check cand->conversion_path. */
bool
xobj_iobj_parameters_correspond (tree fn1, tree fn2)
@@ -1112,29 +1115,10 @@ xobj_iobj_parameters_correspond (tree fn1, tree fn2)
handles xobj parameters of pointer type, we don't have to explicitly
check for that case. */
- /* FIXME:
-
- template<typename>
- struct S;
-
- template<typename>
- struct B {
- int f(this S<void>&) requires true { return 5; }
- };
-
- template<typename>
- struct S : B<void> {
- using B<void>::f;
- int f() { return 10; }
- };
-
- This case is broken, the incomplete type seems to screw with things.
- I'm not sure how to fix that so I'm just noting the issue here, I have a
- feeling it's trivial to do if you know how. */
-
- if (TYPE_MAIN_VARIANT (iobj_param_type)
- != TYPE_MAIN_VARIANT (non_reference (xobj_param)))
+ if (!same_type_ignoring_top_level_qualifiers_p
+ (iobj_param_type, non_reference (xobj_param)))
return false;
+
/* We don't get to bail yet even if we have a by-value xobj parameter,
a by-value xobj parameter can correspond to an iobj parameter provided the
iobj member function is not declared with a reference qualifier.
@@ -8976,9 +8960,9 @@ resolve_address_of_overloaded_function (tree target_type,
documentation for -fms-extensions states it's purpose is to support
the use of microsoft headers. Until otherwise demonstrated, we should
assume xobj member functions are not used in this manner in microsoft
- headers and indiscriminately forbid the incorrect syntax instead of
- supporting it for non-legacy uses. This should hopefully encourage
- conformance going forward.
+ headers and forbid the incorrect syntax instead of supporting it for
+ non-legacy uses. This should hopefully encourage conformance going
+ forward.
This comment is referred to in typeck.cc:cp_build_addr_expr_1. */
if (DECL_IOBJ_MEMBER_FUNCTION_P (fn) && flag_ms_extensions)
/* Early escape. */;
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index da6a08a0304..6a9f03eb8f3 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -3400,8 +3400,7 @@ defaulted_late_check (tree fn)
|| TYPE_REF_IS_RVALUE (fn_obj_ref_type))
return false;
/* If implicit_fn's object parameter is not a pointer, something is not
- right. (Or we have finally changed the type of the iobj parameter
- in iobj member functions.) */
+ right. */
gcc_assert (TYPE_PTR_P (TREE_VALUE (implicit_fn_parms)));
/* Strip the reference/pointer off each object parameter before
comparing them. */
@@ -3422,10 +3421,6 @@ defaulted_late_check (tree fn)
{
error ("defaulted declaration %q+D does not match the "
"expected signature", fn);
- /* FIXME: If the user is defaulting an xobj member function we should
- emit an xobj member function for a signature. When we do this, maybe
- we can just synthesize implicit_fn as an xobj member function and
- avoid the dance in compare_fn_parms. */
inform (DECL_SOURCE_LOCATION (fn),
"expected signature: %qD", implicit_fn);
}
--
2.39.3
[-- Attachment #3: 0002-c-adjust-accessor-fixits-for-explicit-object-parm.patch --]
[-- Type: text/x-patch, Size: 8281 bytes --]
From ae3003b20d3e3ab6e50a6d4f2173e10ad9025135 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Mon, 8 Jan 2024 17:09:54 -0500
Subject: [PATCH 2/2] c++: adjust accessor fixits for explicit object parm
To: gcc-patches@gcc.gnu.org
In a couple of places in the xobj patch I noticed that is_this_parameter
probably wanted to change to is_object_parameter; this implements that and
does the additional adjustments needed to make the accessor fixits handle
xobj parms.
gcc/cp/ChangeLog:
* semantics.cc (is_object_parameter): New.
* cp-tree.h (is_object_parameter): Declare.
* call.cc (maybe_warn_class_memaccess): Use it.
* search.cc (field_access_p): Use it.
(class_of_object_parm): New.
(field_accessor_p): Adjust for explicit object parms.
gcc/testsuite/ChangeLog:
* g++.dg/torture/accessor-fixits-9-xobj.C: New test.
---
gcc/cp/cp-tree.h | 1 +
gcc/cp/call.cc | 3 +-
gcc/cp/search.cc | 19 ++-
gcc/cp/semantics.cc | 14 +++
.../g++.dg/torture/accessor-fixits-9-xobj.C | 119 ++++++++++++++++++
5 files changed, 149 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/torture/accessor-fixits-9-xobj.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index dbc71776be3..f3f265a3db8 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7784,6 +7784,7 @@ extern void finish_handler_parms (tree, tree);
extern void finish_handler (tree);
extern void finish_cleanup (tree, tree);
extern bool is_this_parameter (tree);
+extern bool is_object_parameter (tree);
enum {
BCS_NORMAL = 0,
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 7d3d67600c8..191664ee227 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -10815,8 +10815,7 @@ maybe_warn_class_memaccess (location_t loc, tree fndecl,
be more permissive. */
if (current_function_decl
&& DECL_OBJECT_MEMBER_FUNCTION_P (current_function_decl)
- /* ??? is_object_parameter? */
- && is_this_parameter (tree_strip_nop_conversions (dest)))
+ && is_object_parameter (tree_strip_nop_conversions (dest)))
{
tree ctx = DECL_CONTEXT (current_function_decl);
bool special = same_type_ignoring_top_level_qualifiers_p (ctx, desttype);
diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
index dc76714351f..2b4ed5d024e 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -1737,8 +1737,7 @@ field_access_p (tree component_ref, tree field_decl, tree field_type)
return false;
tree ptr = STRIP_NOPS (TREE_OPERAND (indirect_ref, 0));
- /* ??? is_object_parameter? */
- if (!is_this_parameter (ptr))
+ if (!is_object_parameter (ptr))
return false;
/* Must access the correct field. */
@@ -1818,6 +1817,17 @@ reference_accessor_p (tree init_expr, tree field_decl, tree field_type,
return true;
}
+/* Return the class of the `this' or explicit object parameter of FN. */
+
+static tree
+class_of_object_parm (const_tree fn)
+{
+ tree fntype = TREE_TYPE (fn);
+ if (DECL_XOBJ_MEMBER_FUNCTION_P (fn))
+ return non_reference (TREE_VALUE (TYPE_ARG_TYPES (fntype)));
+ return class_of_this_parm (fntype);
+}
+
/* Return true if FN is an accessor method for FIELD_DECL.
i.e. a method of the form { return FIELD; }, with no
conversions.
@@ -1835,15 +1845,14 @@ field_accessor_p (tree fn, tree field_decl, bool const_p)
if (TREE_CODE (field_decl) != FIELD_DECL)
return false;
- tree fntype = TREE_TYPE (fn);
- if (TREE_CODE (fntype) != METHOD_TYPE)
+ if (!DECL_OBJECT_MEMBER_FUNCTION_P (fn))
return false;
/* If the field is accessed via a const "this" argument, verify
that the "this" parameter is const. */
if (const_p)
{
- tree this_class = class_of_this_parm (fntype);
+ tree this_class = class_of_object_parm (fn);
if (!TYPE_READONLY (this_class))
return false;
}
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 86e1adf1e71..3299e270446 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -12832,6 +12832,20 @@ is_this_parameter (tree t)
return true;
}
+/* As above, or a C++23 explicit object parameter. */
+
+bool
+is_object_parameter (tree t)
+{
+ if (is_this_parameter (t))
+ return true;
+ if (TREE_CODE (t) != PARM_DECL)
+ return false;
+ tree ctx = DECL_CONTEXT (t);
+ return (ctx && DECL_XOBJ_MEMBER_FUNCTION_P (ctx)
+ && t == DECL_ARGUMENTS (ctx));
+}
+
/* Insert the deduced return type for an auto function. */
void
diff --git a/gcc/testsuite/g++.dg/torture/accessor-fixits-9-xobj.C b/gcc/testsuite/g++.dg/torture/accessor-fixits-9-xobj.C
new file mode 100644
index 00000000000..89be978ae8e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/accessor-fixits-9-xobj.C
@@ -0,0 +1,119 @@
+// PR c++/84993
+// { dg-options "-fdiagnostics-show-caret -std=c++23" }
+
+/* Misspelling (by omitting a leading "m_") of a private member for which
+ there's a public accessor.
+
+ We expect a fix-it hint suggesting the accessor. */
+
+class t1
+{
+public:
+ int get_ratio (this const t1& x) { return x.m_ratio; }
+
+private:
+ int m_ratio;
+};
+
+int test (t1 *ptr_1)
+{
+ return ptr_1->ratio; // { dg-error "'class t1' has no member named 'ratio'; did you mean 'int t1::m_ratio'\\? \\(accessible via 'int t1::get_ratio\\(this const t1&\\)'\\)" }
+ /* { dg-begin-multiline-output "" }
+ return ptr_1->ratio;
+ ^~~~~
+ get_ratio()
+ { dg-end-multiline-output "" } */
+}
+
+
+/* Misspelling of a private member for which there's a public accessor.
+
+ We expect a fix-it hint suggesting the accessor. */
+
+class t2
+{
+public:
+ int get_color (this const t2& x) { return x.m_color; }
+
+private:
+ int m_color;
+};
+
+int test (t2 *ptr_2)
+{
+ return ptr_2->m_colour; // { dg-error "'class t2' has no member named 'm_colour'; did you mean 'int t2::m_color'\\? \\(accessible via 'int t2::get_color\\(this const t2&\\)'\\)" }
+ /* { dg-begin-multiline-output "" }
+ return ptr_2->m_colour;
+ ^~~~~~~~
+ get_color()
+ { dg-end-multiline-output "" } */
+}
+
+
+/* Misspelling of a private member via a subclass pointer, for which there's
+ a public accessor in the base class.
+
+ We expect a fix-it hint suggesting the accessor. */
+
+class t3 : public t2 {};
+
+int test (t3 *ptr_3)
+{
+ return ptr_3->m_colour; // { dg-error "'class t3' has no member named 'm_colour'; did you mean 'int t2::m_color'\\? \\(accessible via 'int t2::get_color\\(this const t2&\\)'\\)" }
+ /* { dg-begin-multiline-output "" }
+ return ptr_3->m_colour;
+ ^~~~~~~~
+ get_color()
+ { dg-end-multiline-output "" } */
+}
+
+
+/* Misspelling of a protected member, for which there's isn't a public
+ accessor.
+
+ We expect no fix-it hint; instead a message identifying where the
+ data member was declared. */
+
+class t4
+{
+protected:
+ int m_color; // { dg-message "declared protected here" }
+};
+
+int test (t4 *ptr_4)
+{
+ return ptr_4->m_colour; // { dg-error "'class t4' has no member named 'm_colour'; did you mean 'int t4::m_color'\\? \\(not accessible from this context\\)" }
+ /* { dg-begin-multiline-output "" }
+ return ptr_4->m_colour;
+ ^~~~~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ int m_color;
+ ^~~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+
+/* Misspelling of a private member, for which the accessor is also private.
+
+ We expect no fix-it hint; instead a message identifying where the
+ data member was declared. */
+
+class t5
+{
+ int get_color (this const t5& x) { return x.m_color; }
+ int m_color; // { dg-message "declared private here" }
+};
+
+int test (t5 *ptr_5)
+{
+ return ptr_5->m_colour; // { dg-error "'class t5' has no member named 'm_colour'; did you mean 'int t5::m_color'\\? \\(not accessible from this context\\)" }
+ /* { dg-begin-multiline-output "" }
+ return ptr_5->m_colour;
+ ^~~~~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ int m_color;
+ ^~~~~~~
+ { dg-end-multiline-output "" } */
+}
--
2.39.3
next prev parent reply other threads:[~2024-01-09 21:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-07 0:00 waffl3x
2024-01-09 20:58 ` Jason Merrill
2024-01-09 21:26 ` waffl3x
2024-01-09 21:56 ` Jason Merrill [this message]
2024-01-09 22:34 ` waffl3x
2024-01-09 22:52 ` Jason Merrill
2024-01-09 23:59 ` waffl3x
2024-01-10 17:15 ` Patrick Palka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bdec7223-2d0c-41a6-8283-2d9ba52f0a60@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=waffl3x@protonmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).