Re: [PATCH gcc/* 2/2] gcc: stop using 'int' to represent sets of qualifiers

Jason Merrill <[email protected]> writes:

> It actually wouldn't; in boolean contexts like conditions and && the
> expression is "contextually converted to bool"
> (https://eel.is/c++draft/conv.general#4), which uses an explicit
> conversion operator.

Ah, okay, excellent.  I'll add it.

>> Or, we could settle on using 'nonempty_p' for all these cases.
>> 
>>>> -    unqual_elt = c_build_qualified_type (elt, KEEP_QUAL_ADDR_SPACE (quals));
>>>> +    unqual_elt = c_build_qualified_type (elt,
>>>> +					 quals.without (TYPE_QUAL_ALL));
>>> The quals.without (TYPE_QUAL_ALL) pattern seems awkward, especially given the
>>> semi-ambiguity of "ALL".  How about a without_cv member function?
>> Yes, "ALL" is ambiguous.  I couldn't think of a better term.
>> This only occurred once, which is why I didn't add 'without_cv'.
>> But, I can add it.
>
> It seems useful; wanting the cv-unqualified type comes up a lot.

In the C++ FE, for CV-unqualified versions of a type, the FE uses:

  gcc/cp/tree.cc-tree
  gcc/cp/tree.cc:cv_unqualified (tree type)
  gcc/cp/tree.cc-{
  gcc/cp/tree.cc-  if (type == error_mark_node)
  gcc/cp/tree.cc-    return type;
  gcc/cp/tree.cc-
  gcc/cp/tree.cc-  auto quals = cp_type_quals (type);
  gcc/cp/tree.cc-  quals.remove (TYPE_QUAL_CONST | TYPE_QUAL_VOLATILE);
  gcc/cp/tree.cc-  return cp_build_qualified_type (type, quals);
  gcc/cp/tree.cc-}

... presumably so that it can keep 'restrict' (and _Atomic, but that's
immaterial in C++); could 'without_cv' maybe get confused with that?

(so, maybe 'without_cvra' is better)

Surprisingly, even in the branch where named address space support is present
in C++, I only see one match of 'without (TYPE_QUAL_ALL)', the one
above.

Presumably all the other cases go through 'cv_unqualified'.

>>>> +  /* Returns true if qualifiers in SUBSET can be replaced with qualifiers in
>>>> +     THIS safely.
>>>> +
>>>> +     In general, this means that an object qualified per SUBSET can be used as
>>>> +     if it was qualified per this qualifier set (e.g. 'T' as 'const T', or
>>>> +     'const T' as 'const volatile AS1 T', presuming that AS1 is a superset of
>>>> +     the generic address space).
>>>> +
>>>> +     If NOP_ONLY, return 'true' iff a pointer with a pointee qualified via
>>>> +     SUBSET can be converted into a pointer with a pointee qualified via THIS
>>>> +     (i.e. if a NOP_EXPR conversion would be valid).  In particular, this means
>>>> +     address space mismatches are forbidden.  */
>>>> +  bool can_qualify (qualifier_set subset, bool nop_only = false) const;
>>>
>>> This name and the first paragraph are confusing.  What does "safely" mean?  If
>>> it means there's a C++ standard conversion, let's say that.
>> That's what the second paragraph expands on.
>> I didn't refer to standard conversions because a single qualifier set
>> alone doesn't suffice to decide whether a conversion is allowed (whether
>> 'T * cv *' can be converted to 'const T * cv *' depends on 'cv' rather
>> than just the qualification of the innermost T).
>
> Sure, but doesn't that same argument apply to the second paragraph as
> it is?  In cases where there isn't a standard conversion, it's
> specifically because such a conversion would not be safe.

Well, in the second paragraph, the qualifier set being referred to is
necessarily the toplevel one.  (even though T is a placeholder for any
type, its inner qualifications are the same in both cases and are
obscured by the placeholder itself, so they aren't relevant)

> And we're only dealing with one set of qualifiers here, we could frame it in
> terms of whether cv1 int* can convert to cv2 int* without referring to an
> arbitrary type T.

Yeah, that could work.

>>> How about "superset_of"?
>> Hmm, a superset does not necessarily mean that THIS can be used to
>> qualify an object previously qualified as SUBSET (particularly when
>> NOP_ONLY, or when the sets differ by _Atomic qualification), so it has
>> different semantics.
>
> And yet you call the parameter "SUBSET" :)

Names are hard ;)

Probably, that should be OTHER.

> Maybe "compatible_with" (in a sense similar to C++ reference-compatible)?

Hmm, that seems fine to me, unless it conflicts with some notion of
compatibility in C, at least in the context of qualification.

Assuming it does not, how about:

  /* Returns true if qualifiers in SUBSET can be replaced with
     qualifiers in THIS safely.

     If !POINTEE, such a replacement is safe iff 'cv2 int *' can be
     converted into 'cv1 int *' where cv1 are the qualifiers in THIS and
     cv2 the qualifiers in OTHER.

     Otherwise, such a replacement is safe iff 'cv2 int * const *' can
     be converted into 'cv1 int * const *' with cv1 and cv2 as
     above.  */
  bool compatible_with (qualifier_set other, bool pointee = false) const;
-- 
Arsen Arsenović

-----BEGIN PGP SIGNATURE-----

iQEKBAEWCgCyFiEE/uKz0RP8AKMWLWBhUsKUMB6ixJMFAmqFmzkbFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMiwyXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25z
Lm9wZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRGRUUyQjNEMTEzRkMwMEEzMTYyRDYw
NjE1MkMyOTQzMDFFQTJDNDkzGBxhYXJzZW5vdmljQGJheWxpYnJlLmNvbQAKCRBS
wpQwHqLEk+lOAQD2dUOdsZBhm8DigGQxlFDVh8ZqnoH7poXa8bXA28yzLAD/RNmo
/Oaoox7VI8ivbbWRuklPDmT0fpTLzTcp1CVtPg4=
=iNU3
-----END PGP SIGNATURE-----