Jason Merrill <[email protected]> writes:

> The address space logic needs to respect the 'strict' flags like the rest of
> the function.  So we want to allow as_parm == 0 iff when
> UNIFY_ALLOW_LESS_CV_QUAL, or as_arg == 0 if UNIFY_ALLOW_MORE_CV_QUAL.

Ah, OK.  I've now implemented that:

  static bool
  check_addr_spaces_for_unify (int strict, bool outer,
                               addr_space_t as_arg, addr_space_t as_parm)
  {
    if (as_parm == as_arg)
      return true;

    if (!ADDR_SPACE_GENERIC_P (as_parm) && !ADDR_SPACE_GENERIC_P (as_arg))
      {
        /* We can't unify different non-generic address spaces at all, except on
           the outer layer, if we're permitting more CV-qualification.  */
        if (outer
            && targetm.addr_space.subset_p (as_arg, as_parm)
            && (strict & (UNIFY_ALLOW_MORE_CV_QUAL | 
UNIFY_ALLOW_OUTER_MORE_CV_QUAL)))
          return true;

        return false;
      }

    /* At this point, exactly one of PARM or ARG has an address space.  */
    if (!(strict & (UNIFY_ALLOW_MORE_CV_QUAL | UNIFY_ALLOW_OUTER_MORE_CV_QUAL))
        && ADDR_SPACE_GENERIC_P (as_arg))
      /* ARG lacks an address space, but PARM has it.  This means PARM is more
         qualified, but we don't allow that.  */
      return false;

    if (!(strict & (UNIFY_ALLOW_LESS_CV_QUAL | UNIFY_ALLOW_OUTER_LESS_CV_QUAL))
        && ADDR_SPACE_GENERIC_P (as_parm))
      /* Conversely, ARG is more qualified.  */
      return false;

    return true;
  }

  [...]

  static int
  check_cv_quals_for_unify (int strict, tree arg, tree parm)
  {
    int arg_quals = CLEAR_QUAL_ADDR_SPACE (cp_type_quals (arg));
    int parm_quals = CLEAR_QUAL_ADDR_SPACE (cp_type_quals (parm));
  
    addr_space_t as_arg = DECODE_QUAL_ADDR_SPACE (cp_type_quals (arg));
    addr_space_t as_parm = DECODE_QUAL_ADDR_SPACE (cp_type_quals (parm));

    if (!check_addr_spaces_for_unify (strict, false, as_arg, as_parm))
      /* Address spaces cannot be unified.  */
      return 0;


> And I don't think we want to consider superset here, just compare
> as_parm == as_arg.  Deduction is stricter than conversion.

We do want to permit it on the outer level, due to
<https://eel.is/c++draft/temp.deduct.call#4.2>, as in the following testcase:

  template<typename T>
  T foo(__super T *x);

  void
  bar(__sub int *y)
  { foo(y); }

... where __super and __sub are a pair of address spaces s.t. __super ⊃
__sub.  There's a qualification conversion from __sub int* to __super T*
[with T = int].

I'm not sure this is the best way to go about it, but I've implemented
that behavior by teaching 'unify' to chuck out the ARG address space of
the toplevel pointer type as such (ergo the seemingly useless OUTER
parameter above), as that means that PARM can always be considered more
qualified than ARG:

  case POINTER_TYPE:
    {
      if (!TYPE_PTR_P (arg))
        return unify_type_mismatch (explain_p, parm, arg);

      /* [temp.deduct.call]

         A can be another pointer or pointer to member type that can
         be converted to the deduced A via a qualification
         conversion (_conv.qual_).

         We pass down STRICT here rather than UNIFY_ALLOW_NONE.
         This will allow for additional cv-qualification of the
         pointed-to types if appropriate.  */

      if (TREE_CODE (TREE_TYPE (arg)) == RECORD_TYPE)
        /* The derived-to-base conversion only persists through one
           level of pointers.  */
        strict |= (strict_in & UNIFY_ALLOW_DERIVED);

      auto arg_pointee = TREE_TYPE (arg);
      if (strict_in & UNIFY_ALLOW_OUTER_LEVEL)
        {
          /* If we're in the outermost layer of a deduction, and address spaces
             differ, this may still be acceptable as a result of a
             qualification conversion existing.

             However, there's no qualification conversions for address space
             differences lower than one level deep, so we cannot simply
             handle this case when unifying the two pointee types.

             We'll handle this by stripping off the address space of ARG's
             pointee here, if its address space is a subset of PARMs.  This
             will permit check_cv_quals_for_unify to succeed in case such a
             conversion exists, or fail otherwise.  */

          auto parm_pointee_as = TYPE_ADDR_SPACE (TREE_TYPE (parm));
          auto arg_pointee_as = TYPE_ADDR_SPACE (TREE_TYPE (arg));
          auto arg_pointee_quals = cp_type_quals (TREE_TYPE (arg));

          if (parm_pointee_as == arg_pointee_as)
            /* All OK.  */;
          else if (check_addr_spaces_for_unify (strict, true,
                                                arg_pointee_as,
                                                parm_pointee_as))
            /* We're permitted to remove the addr-space difference.  Doing
               this will allow deducing to T without the address-space, and
               then applying a qualifying conversion to it.  */
            arg_pointee = (cp_build_qualified_type
                           (arg_pointee,
                            CLEAR_QUAL_ADDR_SPACE (arg_pointee_quals)));
        }

      return unify (tparms, targs, TREE_TYPE (parm),
                    arg_pointee, strict, explain_p);
    }

AFAICT, rather than attempting unification quite permissively and then
throwing away a successful deduction in case [temp.deduct.call]p4 fails,
we "inline" the qualification test into the unification process itself.

>> @@ -11963,6 +12126,7 @@ comp_ptr_ttypes_real (tree to, tree from, int constp)
>>        if (!at_least_as_qualified_p (from, to))
>>          {
>>            if (constp == 0)
>> +            /* I'm not sure what this check is meant to be covering.  */
>>              return false;
> Under https://eel.is/c++draft/conv#qual you can add cv-quals on an inner level
> if the outer levels are all const, e.g. int** -> int const* const*.  If constp
> == 0, the outer levels aren't all const, so adding cv-quals is not allowed.
>
> I don't think we want to extend that rule to address spaces, so your
> is_toplevel seems right.
>
> Feel free to add a comment.

Ah, I see.  It's not immediately clear that the only valid states for
CONSTP are 1 (all layers so far had const), 0 (some layer lacked const,
never passed in by callers), and -1 (only return 1 for proper subsets),
especially as the function comment explicitly says "If CONSTP is
positive, then all outer pointers have been const-qualified" implying
that the caller is partially responsible for making sure this condition
holds, and that any positive value rather than just 1 is permitted.

>> @@ -11952,6 +12101,20 @@ comp_ptr_ttypes_real (tree to, tree from, int 
>> constp)
>>                         TYPE_OFFSET_BASETYPE (to)))
>>      return false;
>>  +      /* We want to permit a toplevel address space change, but only into a
>> +     subset address space.  TODO(arsen): this change has an effect on
>> +     compare_ics through comp_cv_qual_signature, but I have no idea what
>> +     that effect is yet.  */
>> +      if (auto as_to = TYPE_ADDR_SPACE (to), as_from =  TYPE_ADDR_SPACE 
>> (from);
>
> init-statements are a C++17 feature and we're still limited to C++14.

Ah, right.  Will fix all instances.

>> +      as_to != as_from)
>> +    {
>> +      if (!is_toplevel)
>> +        return false;
>> +
>> +      if (!targetm.addr_space.subset_p (as_from, as_to))
>
> This doesn't handle the constp == -1 case where we want to return
> false if each is a subset of the other.

Sure, but doesn't the pair of at_least_as_qualified_p checks later down
(and that was mentioned above)?  Since...

  at_least_as_qualified_p (to, from)
  ⇒ targetm.addr_space.subset_p (from_as, to_as)

... and both permutations are tested with that predicate, ergo both
permutations of SUBSET_P are also tested.

> (which is gratuitously obscure, the parameter should really be a bool
> called something like "proper" and constp be a local variable)

I wouldn't mind making this change also, if you want.  I left a comment,
but this is still quite confusing.
-- 
Arsen Arsenović

Attachment: signature.asc
Description: PGP signature

Reply via email to