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.
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.
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, spelling out OBJECT to avoid visual confusion
with either IOBJ/XOBJ.
Renaming the old macro doesn't need to happen in this patch, but adding
the new macro should.
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.
Other than this, lambdas are not yet supported,
The error I'm seeing with the lambda testcase is "explicit object member
function cannot have cv-qualifier". To avoid that, in
cp_parser_lambda_declarator_opt you need to set quals to
TYPE_UNQUALIFIED around where we do that for mutable lambdas.
and there is some outstanding
odd behavior where invalid calls to operators are improperly accepted. The
test for this utilizes requires expressions to operate though so it's possible
that the problems originate from there, but I have a hunch they aren't
responsible. See explicit-obj-ops-requires-mem.C and
explicit-obj-ops-requires-non-mem.C for those tests.
I think that's the same issue as by-value1.C above.
Though you're also missing a couple of semicolons on
+ RRef&& operator()(this RRef&& self) { return static_cast<RRef&&>(self) }
+ RRef&& operator[](this RRef&& self) { return static_cast<RRef&&>(self) }
@@ -7164,6 +7164,12 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue,
tsubst_flags_t complain)
&& !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 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);
Specifically, you're missing the SET_EXPR_LOCATION at the end of the
function. Maybe break here instead of returning?
+// missing test for three-way-compare (I don't know how to write it)
+// missing test for ->* (I don't know how to write it)
Following the same pattern seems to work for me, e.g. (OPERAND) <=> 0
and (OPERAND) ->* 0.
+template<typename T = void>
+void do_calls()
You probably want to instantiate the templates in these testcases to
make sure that works.
+// It's very hard to test for incorrect successes without requires, and by
extension a non dependent variable
+// so for the time being, there are no non dependent tests invalid calls.
I don't understand the problem, you can have requires-expressions in
non-dependent context.
Jason
From 07b007a44a277f7b5cde69f5e54c2be336dfca1b Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Thu, 9 Nov 2023 16:34:55 -0500
Subject: [PATCH] gcc/testsuite/ChangeLog:
To: gcc-patches@gcc.gnu.org
* g++.dg/cpp23/explicit-obj-ops-non-mem.h: Add <=> and ->*.
---
.../g++.dg/cpp23/explicit-obj-ops-non-mem.h | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/g++.dg/cpp23/explicit-obj-ops-non-mem.h b/gcc/testsuite/g++.dg/cpp23/explicit-obj-ops-non-mem.h
index b94e56b1dd6..2a8d46ed2c6 100644
--- a/gcc/testsuite/g++.dg/cpp23/explicit-obj-ops-non-mem.h
+++ b/gcc/testsuite/g++.dg/cpp23/explicit-obj-ops-non-mem.h
@@ -1,9 +1,6 @@
-// missing test for three-way-compare (I don't know how to write it)
-// missing test for ->* (I don't know how to write it)
-
// tests for ops that must be member functions are seperate
-#define MAKE_STRUCT_OPS(TYPE) \
+#define MAKE_STRUCT_OPS(TYPE) \
TYPE operator+=(this TYPE self, int) { return self; } \
TYPE operator-=(this TYPE self, int) { return self; } \
TYPE operator*=(this TYPE self, int) { return self; } \
@@ -39,7 +36,9 @@
TYPE operator>(this TYPE self, int) { return self; } \
TYPE operator<=(this TYPE self, int) { return self; } \
TYPE operator>=(this TYPE self, int) { return self; } \
+ TYPE operator<=>(this TYPE self, int) { return self; } \
TYPE operator*(this TYPE self) { return self; } \
+ TYPE operator->*(this TYPE self, int) { return self; } \
TYPE operator&(this TYPE self) { return self; } \
TYPE operator,(this TYPE self, int) { return self; }
@@ -105,8 +104,10 @@ struct Deduced {
template<typename Self> Self&& operator>(this Self&& self, int) { return static_cast<Self&&>(self); }
template<typename Self> Self&& operator<=(this Self&& self, int) { return static_cast<Self&&>(self); }
template<typename Self> Self&& operator>=(this Self&& self, int) { return static_cast<Self&&>(self); }
+ template<typename Self> Self&& operator<=>(this Self&& self, int) { return static_cast<Self&&>(self); }
template<typename Self> Self&& operator*(this Self&& self) { return static_cast<Self&&>(self); }
+ template<typename Self> Self&& operator->*(this Self&& self, int) { return static_cast<Self&&>(self); }
template<typename Self> Self&& operator&(this Self&& self) { return static_cast<Self&&>(self); }
template<typename Self> Self&& operator,(this Self&& self, int) { return static_cast<Self&&>(self); }
};
@@ -151,8 +152,10 @@ struct Deduced {
(OPERAND) > 0; \
(OPERAND) <= 0; \
(OPERAND) >= 0; \
+ (OPERAND) <=> 0; \
\
*(OPERAND); \
+ (OPERAND) ->* 0; \
&(OPERAND); \
(OPERAND), 0;
@@ -196,7 +199,9 @@ struct Deduced {
static_assert(__is_same(CORRECT_TYPE, decltype((OPERAND) > 0))); \
static_assert(__is_same(CORRECT_TYPE, decltype((OPERAND) <= 0))); \
static_assert(__is_same(CORRECT_TYPE, decltype((OPERAND) >= 0))); \
+ static_assert(__is_same(CORRECT_TYPE, decltype((OPERAND) <=> 0))); \
\
static_assert(__is_same(CORRECT_TYPE, decltype(*(OPERAND)))); \
+ static_assert(__is_same(CORRECT_TYPE, decltype((OPERAND) ->* 0))); \
static_assert(__is_same(CORRECT_TYPE, decltype(&(OPERAND)))); \
static_assert(__is_same(CORRECT_TYPE, decltype((OPERAND), 0)));
--
2.39.3
