On 12/10/23 10:22, waffl3x wrote:
On Friday, December 8th, 2023 at 12:25 PM, Jason Merrill <ja...@redhat.com>
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<tree,
va_gc> 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 <typename T = int>
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.
I agree, I've suggested amending 2789 to use "corresponding object
parameter".
@@ -8727,21 +8882,42 @@ resolve_address_of_overloaded_function (tree
target_type,
/* Good, exactly one match. Now, convert it to the correct type. */
fn = TREE_PURPOSE (matches);
- if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
- && !(complain & tf_ptrmem_ok) && !flag_ms_extensions)
+ if (DECL_OBJECT_MEMBER_FUNCTION_P (fn)
+ && !(complain & tf_ptrmem_ok))
{
- static int explained;
-
- if (!(complain & tf_error))
+ /* For iobj member functions, if if -fms_extensions was passed in, this
+ is not an error, so we do nothing. It is still an error regardless
+ for xobj member functions though, as it is a new feature we
+ (hopefully) don't need to support the behavior. */
Unfortunately, it seems that MSVC extended their weirdness to xobj fns,
so -fms-extensions should as well.
https://godbolt.org/z/nfvn64Kx5
Ugh, what a shame. We don't have to support it though do we? The
documentation for -fms-extensions seems to state that it's for
microsoft headers, if they aren't using xobj parameters in their
headers as of now, and I can't see them doing so for the near future,
it should be fine to leave this out still shouldn't it?
The code will be simpler if we can't though so I reckon it's win/win
whichever way we choose.
I think let's go with the simpler code, it doesn't seem particularly
useful to split hairs on this obscure option.
We could also change dependent_type_p to the more specific
WILDCARD_TYPE_P, I think, both here and just above.
I don't understand the significance of this but I'll trust there is
one. Better to be consistent with your other changes anyway.
FYI a "wildcard" type is e.g. a template type parm, typename, or
decltype: types that could be replaced by a type of any form (and
qualification), unlike e.g. A<T> which is also dependent, but won't turn
const at instantiation time (if A is a class template).
+ The name "nonstatic" is no longer accurate with the addition of
+ xobj member functions, should we change the name? */
bool
invalid_nonstatic_memfn_p (location_t loc, tree expr, tsubst_flags_t complain)
This rule still applies to all non-static member functions:
https://eel.is/c++draft/expr.ref#6.3.2
Alright I see where my mistake is now, the comment regarding .* and ->*
threw me for a loop. Even so, I figured since we renamed
DECL_NONSTATIC_MEMBER_FUNCTION_P we might be better off renaming other
things too. So in this case, invalid_object_memfn_p instead of
invalid_nonstatic_memfn_p.
Obviously changing the errors and warnings is a different beast and
probably should be left alone.
I wouldn't bother renaming other things unnecessarily, it just seemed
important to rename the predicate because it was being used for what are
now two different meanings.
Here's a patch to adjust all the remaining
DECL_NONSTATIC_MEMBER_FUNCTION_P. With this patch -diagnostic7.C gets
the old address of non-static diagnostic, I think correctly, so I'm not
sure we still need the BASELINK change in cp_build_addr_expr_1?
Thanks, I'll evaluate the BASELINK change to see if you're right.
Hopefully you are, that would simplify things.
Is there anything special I need to do when adding this patch? I was
worried I'm supposed to maintain it's origin in it's own commit or
something. Well, with that said it's probably still something I should
learn to do anyway, I'm just trying really hard to put it off until the
patch is done.
It's OK to integrate it into your patch if that's easier for you.
Jason
