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:
From 5a6d3b1737843aa64d83ffc5d639fa0afa5d8318 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@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,
/* 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
From ae3003b20d3e3ab6e50a6d4f2173e10ad9025135 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@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
