Re: [PATCH v6 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[Jason Merrill <jason@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5782 bytes --]

On 11/27/23 00:35, waffl3x wrote:
> I think this is cleaned up enough to be presentable. Bootstrapped but
> not tested but I don't think I changed anything substantial. I am
> running tests right now and will report if anything fails. I have not
> fixed the problem in tsubst_lambda_expr that we talked about, that will
> be first on my list tomorrow. While writing/cleaning up tests I had to
> investigate some things, one of which is calling an overloaded
> function, where one of the candidates are introduced by a using
> declaration, is considered ambiguous. I haven't narrowed down the case
> for this yet so I don't know if it's related to xobj member
> functions/lambda with xobj parameters or not. I had to pull a few tests
> because of it though.
> 
> I did not get as much done as I would have hoped today. This really
> just serves as a small progress update. Once again, todo is the issue
> you raised in tsubst_lambda_expr, and fixing handling of captures when
> a const xobj parameter is deduced in a lamdba call operator.

> +#define DECL_IOBJ_MEMBER_FUNC_P(NODE) \
> +#define DECL_XOBJ_MEMBER_FUNC_P(NODE)                  \
> +#define DECL_OBJECT_MEMBER_FUNC_P(NODE) \

Let's use the full word FUNCTION in these macros for consistency with 
DECL_STATIC_FUNCTION_P.

> @@ -6544,7 +6544,7 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
>        tree fn_first_arg = NULL_TREE;
>        const vec<tree, va_gc> *fn_args = args;
>  
> -      if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn))
> +      if (DECL_OBJECT_MEMBER_FUNC_P (fn))
>         {
>           /* Figure out where the object arg comes from.  If this
>              function is a non-static member and we didn't get an

Hmm, that this function explicitly pulls out the object argument into 
first_arg strengthens your earlier argument that we shouldn't bother 
trying to handle null first_arg.  But let's not mess with that in this 
patch.

> +      val = handle_arg(TREE_VALUE (parm),

Missing space.

>           /* We know an iobj parameter must be a reference. If our xobj                                                             
>              parameter is a pointer, we know this is not a redeclaration.                                                           
>              This also catches array parameters, those are pointers too.  */
>           if (TYPE_PTR_P (xobj_param))
>             continue;

Handling pointers specifically here seems unnecessary, they should be 
rare and will be handled by the next check for unrelated type.

>              dealing with a by-value xobj parameter we can procede following                                                        

"proceed"

>    /* An operator function must either be a non-static member function
>       or have at least one parameter of a class, a reference to a class,
>       an enumeration, or a reference to an enumeration.  13.4.0.6 */
> -  if (! methodp || DECL_STATIC_FUNCTION_P (decl))
> +  /* This can just be DECL_STATIC_FUNCTION_P (decl) I think?  */
> +  if ((!methodp && !DECL_XOBJ_MEMBER_FUNC_P (decl))
> +      || DECL_STATIC_FUNCTION_P (decl))

No, it also needs to be true for non-members; rather, I think it can 
just be if (!DECL_OBJECT_FUNCTION_P (decl))

> +  if (xobj_param)
> +    {
> +      quals = TYPE_UNQUALIFIED;
> +      if (TYPE_REF_P (xobj_param)
> +         && !(cp_type_quals (TREE_TYPE (xobj_param)) & TYPE_QUAL_CONST))
> +        LAMBDA_EXPR_MUTABLE_P (lambda_expr) = 1;
> +    }

I don't think we want to mess with MUTABLE_P here.  But then 
capture_decltype needs to be fixed to refer to the type of the object 
parameter rather than MUTABLE_P.

Actually, I think we can do away with the MUTABLE_P flag entirely.  I'm 
going to push a patch to do that.

> -       if (!LAMBDA_EXPR_STATIC_P (lambda_expr))
> +       if (!LAMBDA_EXPR_STATIC_P (lambda_expr)
> +           && !DECL_XOBJ_MEMBER_FUNC_P (fco))

This could be if (DECL_IOBJ_MEMBER_FUNCTION_P (fco))

> -  if (closure && !DECL_STATIC_FUNCTION_P (t))
> +  if (closure && DECL_IOBJ_MEMBER_FUNC_P (t) && !DECL_STATIC_FUNCTION_P (t))

This shouldn't need to still check DECL_STATIC_FUNCTION_P.

> +  /* We don't touch a lambda's func when it's just trying to create the
> +     closure type.  */

We need to check it somewhere, currently this crashes:

template <class T> void f()
{
   int i;
   [=](this T&& self){ return i; }(); // error, unrelated
}
int main() { f<int>(); }

> @@ -3691,18 +3691,7 @@ build_min_non_dep_op_overload (enum tree_code op,
>    releasing_vec args;
>    va_start (p, overload);
>  
> -  if (TREE_CODE (TREE_TYPE (overload)) == FUNCTION_TYPE)
> -    {
> -      fn = overload;
> -      if (op == ARRAY_REF)
> -       obj = va_arg (p, tree);
> -      for (int i = 0; i < nargs; i++)
> -       {
> -         tree arg = va_arg (p, tree);
> -         vec_safe_push (args, arg);
> -       }
> -    }

Maybe change the test to !DECL_OBJECT_MEMBER_FUNC_P to avoid reordering 
the cases?

> @@ -15402,6 +15450,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain,
>                   gcc_checking_assert (TYPE_MAIN_VARIANT (TREE_TYPE (ve))
>                                        == TYPE_MAIN_VARIANT (type));
>                 SET_DECL_VALUE_EXPR (r, ve);
> +               if (is_capture_proxy (t))
> +                 type = TREE_TYPE (ve);

That should have close to the same effect as the lambda_proxy_type 
adjustment I was talking about, since that function basically returns 
the TREE_TYPE of the COMPONENT_REF.  But the underlying problem is that 
finish_non_static_data_member assumes that 'object' is '*this', for 
which you can trust the cv-quals; for auto&&, you can't. 
capture_decltype has the same problem.  I'm attaching a patch to address 
this in both places.

Jason

[-- Attachment #2: wildcard.patch --]
[-- Type: text/x-patch, Size: 2113 bytes --]

commit 520217cc15315fe1cc58dff23c65bb3cdc28dc56
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Nov 29 21:51:36 2023 -0500

    wildcard

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 7946f0be77b..e0cb8a1b04f 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -2265,6 +2265,10 @@ finish_non_static_data_member (tree decl, tree object, tree qualifying_scope,
       else if (PACK_EXPANSION_P (type))
 	/* Don't bother trying to represent this.  */
 	type = NULL_TREE;
+      else if (WILDCARD_TYPE_P (TREE_TYPE (object)))
+	/* We don't know what the eventual quals will be.  This can happen when
+	   called from build_capture_proxy for an explicit object lambda.  */
+	type = NULL_TREE;
       else
 	{
 	  /* Set the cv qualifiers.  */
@@ -11709,6 +11713,7 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
      A<decltype(sizeof(T))>::U doesn't require 'typename'.  */
   if (instantiation_dependent_uneval_expression_p (expr))
     {
+    dependent:
       type = cxx_make_type (DECLTYPE_TYPE);
       DECLTYPE_TYPE_EXPR (type) = expr;
       DECLTYPE_TYPE_ID_EXPR_OR_MEMBER_ACCESS_P (type)
@@ -11883,7 +11888,11 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
       if (outer_automatic_var_p (STRIP_REFERENCE_REF (expr))
 	  && current_function_decl
 	  && LAMBDA_FUNCTION_P (current_function_decl))
-	type = capture_decltype (STRIP_REFERENCE_REF (expr));
+	{
+	  type = capture_decltype (STRIP_REFERENCE_REF (expr));
+	  if (!type)
+	    goto dependent;
+	}
       else if (error_operand_p (expr))
 	type = error_mark_node;
       else if (expr == current_class_ptr)
@@ -12821,7 +12830,8 @@ capture_decltype (tree decl)
     {
       int quals = cp_type_quals (type);
       tree obtype = TREE_TYPE (DECL_ARGUMENTS (current_function_decl));
-      gcc_checking_assert (!WILDCARD_TYPE_P (non_reference (obtype)));
+      if (WILDCARD_TYPE_P (non_reference (obtype)))
+	return NULL_TREE;
       if (INDIRECT_TYPE_P (obtype))
 	quals |= cp_type_quals (TREE_TYPE (obtype));
       type = cp_build_qualified_type (type, quals);