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. 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? > @@ -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 > + /* I'm keeping it more basic for now. */ OK, this comment can go. > @@ -15502,9 +15627,10 @@ void > grok_special_member_properties (tree decl) > { > tree class_type; > - > + /* I believe we have to make some changes in here depending on the outcome > + of CWG2586. */ As mentioned above, CWG2586 is resolved. Be sure to scroll down to the approved resolution, or refer to the working draft. https://cplusplus.github.io/CWG/issues/2586.html > @@ -11754,8 +11754,16 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tre > else if (cxx_dialect < cxx23) > omitted_parms_loc = cp_lexer_peek_token (parser->lexer)->location; > > + /* Review note: I figured I might as well update the comments since I'm here. > + There are also some additions to the below. */ Great, this comment can go. > + /* [expr.prim.lambda.general-4] > + If the lambda-declarator contains an explicit object parameter > + ([dcl.fct]), then no lambda-specifier in the lambda-specifier-seq > + shall be mutable or static. */ > + if (lambda_specs.storage_class == sc_mutable) > + { > + auto_diagnostic_group d; > + error_at (lambda_specs.locations[ds_storage_class], > + "% lambda specifier " > + "with explicit object parameter"); > + /* Tell the user how to do what they probably meant, maybe fixits > + would be apropriate later? */ "appropriate" > + if (!dependent_type_p (non_reference (param_type))) > + /* If we are given a non-dependent type we will have already given > + a diagnosis that the following would contradict with. */; Only if the lambda has captures, though? We could also change dependent_type_p to the more specific WILDCARD_TYPE_P, I think, both here and just above. > + else if (!TYPE_REF_P (param_type)) > + inform (DECL_SOURCE_LOCATION (xobj_param), > + "the passed in closure object will not be mutated because " > + "it is taken by copy/move"); "by value" > @@ -3092,7 +3093,31 @@ finish_this_expr (void) > return rvalue (result); > > tree fn = current_nonlambda_function (); > - if (fn && DECL_STATIC_FUNCTION_P (fn)) > + if (fn && DECL_XOBJ_MEMBER_FUNCTION_P (fn)) > + { > + auto_diagnostic_group d; > + error ("% is unavailable for explicit object member " > + "functions"); > + /* Doing a fixit here is possible, but hard, might be worthwhile > + in the future. */ > + tree xobj_parm = DECL_ARGUMENTS (fn); > + gcc_assert (xobj_parm); > + tree parm_name = DECL_NAME (xobj_parm); > + if (parm_name) > + inform (DECL_SOURCE_LOCATION (xobj_parm), > + "use explicit object parameter %qD instead", > + parm_name); > + else > + { > + gcc_rich_location xobj_loc (DECL_SOURCE_LOCATION (xobj_parm)); > + /* This doesn't work and I don't know why. I'll probably remove it > + before the final version. */ > + xobj_loc.add_fixit_insert_after (" self"); > + inform (DECL_SOURCE_LOCATION (xobj_parm), > + "name and use the explicit object parameter instead"); Now seems to be the time to either fix or remove it. > @@ -12811,9 +12836,11 @@ capture_decltype (tree decl) > if (WILDCARD_TYPE_P (non_reference (obtype))) > /* We don't know what the eventual obtype quals will be. */ > return NULL_TREE; > - int quals = cp_type_quals (type); > - if (INDIRECT_TYPE_P (obtype)) > - quals |= cp_type_quals (TREE_TYPE (obtype)); > + /* This change possibly conflicts with another patch that is currently > + in flight. (Patrick Palka's PR83167 patch) I am just changing it for > + now to satisfy my tests. */ > + int const quals = cp_type_quals (type) > + | cp_type_quals (TREE_TYPE (obtype)); Doesn't TREE_TYPE (obtype) break for by-value xob? In that case I think we'd want cp_type_quals (obtype). > + 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 > @@ -2352,7 +2355,7 @@ invalid_nonstatic_memfn_p (location_t loc, tree expr, tsubst_flags_t complain) > if (is_overloaded_fn (expr) && !really_overloaded_fn (expr)) > expr = get_first_fn (expr); > if (TREE_TYPE (expr) > - && DECL_NONSTATIC_MEMBER_FUNCTION_P (expr)) > + && DECL_IOBJ_MEMBER_FUNCTION_P (expr)) ...and so I think this should be _OBJECT_. https://godbolt.org/z/r6v4e1ePP 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? Jason