pat-rogers / Ada-202x-WG9-Informal-Review

This is the place for WG 9 members to submit informal comments on the 202x source document. (This is not the formal ballot that WG 9 will hold later in the process.)
0 stars 0 forks source link

6.4.1(5.1/5) through 6.4.1(5.4/4) missing legality rule regarding inherited routine with an out parameter #5

Open pat-rogers opened 3 years ago

pat-rogers commented 3 years ago

Paragraphs 6.4.1(5.1/5) through 6.4.1(5.4/4) define a Legality check for some scalar view conversions passed to OUT mode parameters.

When a type is derived, any inherited subprograms are called with the actual parameters of the new type view converted to the parent type. This can cause an anomaly, in that an explicit call would be illegal while calling an inherited routine (which makes the same call implicitly) is legal.

Tucker notes that 6.4.1(15.1/5) says that this view conversion raises Program_Error. Therefore, there is no language bug in that code that does not follow the principles cannot be executed. OTOH, the rule Tucker refers to was only intended to be used in generic bodies (and there is an AARM note to that effect). So implementers and users alike are likely to be confused.

Moreover, we have a Language Design Principle (admittedly, not always followed) that a construct which will always raise an exception should be detected at compile-time. So it seems that a Legality Rule is missing here.

Suggested fix: Add a Legality Rule to reject any call on an inherited routine with an out parameter where the implied view conversion would be illegal.

Example: Here is an example of this situation:

package P1 is type Moddy is mod 2**16;

  procedure Convert (Ival : in Integer; Mval : out Moddy);

end P1;

with P1; package P2 is type IModdy is new P1.Moddy with Default_Value => 65535; -- [A] -- Convert inherited here. end P2;

with P1, P2; procedure Tester is V : P2.IModdy; begin P1.Convert (10, P1.Moddy(V)); -- [B] P2.Convert (10, V); -- [C] end Tester;

[C] is implemented as [B] -- 3.4(27/2).

But [B] is illegal by 6.4.1(5.1-4/5) -- the operand type has a Default_Value and the target type does not.

The equivalence is only for Dynamic Semantics, so the Legality Rule does not apply. But 6.4.1(15.1/5), so [C] is required to raise Program_Error.

              Randy Brukardt, ARG Editor.
pat-rogers commented 3 years ago

It would perhaps be better to require such primitives to be overridden, since having a primitive that is illegal to call might have cascading weird effects. On the other hand, this only comes up with non-tagged types, so there is no dispatching to worry about, and ancestor primitives re-emerge in a generic, so perhaps I am merely sowing Fear, Uncertainty, and Doubt.

It still seems friendlier, though, to require overriding. You could of course override with a subprogram that always raised Program_Error if so desired, but at least you are being explicit about it. -Tuck

ARG-Editor commented 3 years ago

I didn't suggest requiring overriding, as that is something that currently happens only for tagged types. As such, adding it for an unlikely case for untagged types could be a substantial implementation burden. (If tagged derivation is separately implemented from untagged derivation, then one would need to implement a new mechanism for untagged types.) I do agree it would be friendlier to a user, but only in unusual circumstances (and it would be incompatible with Ada 2012 in those circumstances).

I don't think there would be any "cascading weird effects", as you note that there isn't any effect in generics.

sttaft commented 3 years ago

An alternative would be to add yet another restriction on applying a representation clause to a type with primitives. It used to be completely illegal, and now we are beginning to find out why... ;-) But at least we are certain we are keeping all of the "hair" in the same place, namely, in limiting how far we loosen this rule. I'll see if I can come up with wording for the two different approaches -- disallowing the call, or disallowing the representation change -- and I'll add it as another comment.

ARG-Editor commented 3 years ago

It would seem weird to add something to 13.1(10/5) about a specific aspect used with a specific kind of parameter - it doesn't seem general enough. But I suppose we could have a wart rule in 3.5 specifically to disallow such Default_Values. That seems less friendly than just disallowing uses of such inherited routines, but it's probably not a huge issue either way.

sttaft commented 3 years ago

Steve Baird writes: Given our recent discussion about inherited subprograms with aliased untagged parameters, it sounds like we are already planning on adding new cases to 13.1(10) for other reasons. So I would prefer to address this problem there.

sttaft commented 3 years ago

We are simplifying the current rule in 3.5, so perhaps we could make it more complex again:

If a derived type [with no primitive subprograms] inherits a boolean Default_Value aspect, the aspect may be specified to have any value for the derived type. {If a derived type T does not inherit a Default_Value aspect, it shall not specify such an aspect if it inherits a primitive subprogram that has a parameter of type T of mode OUT.

AARM Reason: This is to avoid violating the rules specified in 6.4.1 about view conversions of OUT parameters with a specified Default_Value aspect.}

The rules in 6.4.1(5.1/5) through 6.4.1(5.4/4) only disallow cases where one type has the Default_Value and the other doesn't, or where they both have it but don't have a common ancestor. Clearly a derived type and its parent have a common ancestor, and clearly you can't get rid of a Default_Value aspect when you have it, so the only issue is adding a Default_Value aspect.

ARG-Editor commented 3 years ago

This makes more sense than attempting to do something in 13.1(10), since this is a special case that applies to only one aspect, while 13.1 is about general rules that apply to large categories of aspects.

ARG-Editor commented 3 years ago

This rule has the advantage that it could be compatibly relaxed in the future (perhaps to "require overriding"), if it proves to be a problem. Other rules would probably require an incompatible change to be made more friendly.

I've added this to the fixup AI, AI12-0427-1.