[m-rev.] for review: don't allow nondefault mode functions in terms

Mark Brown mark at mercurylang.org
Sat Oct 31 20:33:36 AEDT 2015


On Sat, Oct 31, 2015 at 6:10 PM, Zoltan Somogyi
<zoltan.somogyi at runbox.com> wrote:
> On Sat, 31 Oct 2015 17:02:32 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>> > What happens if it is preserved for a while, and then it is lost?
>> > Uses during the first period are fine, uses during the second period
>> > cause a crash. At the moment, we have no way to prevent that loss
>> > of information. Maybe we should consider changing the rules to
>> > outlaw not putting such nonstandard signature functions into terms,
>> > but losing the higher inst info on terms containing higher order values.

Yes, that's what I'd like.

>> > Unfortunately, I am pretty sure that implementing that would be
>> > a massive pain with the current mode system implementation.
>>
>> For now, as it certainly isn't acceptable for the compiler to be
>> accepting programs that cause segmentation faults, we need to go with
>> Peter's suggestion: disallow the construction of terms with a
>> non-default mode function _as a limitation of the current
>> implementation_.

I'll only accept that if the bug really is too hard to fix otherwise.

But my take on it is that non-default func insts should not be
considered ground, for the purposes of inst_is_ground in inst_match.m.
This is consistent with the definitions of inst_match_{initial,final}
when the second argument is ground(_, none).

The below diff implements what I mean.

>
> By that, do you mean that the diff is ok, provided we also document
> the new limitation?
>
>> The larger language design issue raised here can't be resolved with the
>> current implementation of mode checking.
>
> I am not sure about a flat "can't be resolved", but I certainly agree with
> "can't be resolved without massive pain".

I haven't bootchecked the following diff yet, which I will be doing
shortly. Does anyone see any problem in principle?

Cheers,
Mark.

diff --git a/compiler/inst_match.m b/compiler/inst_match.m
index b7bef2f..bf7f03d 100644
--- a/compiler/inst_match.m
+++ b/compiler/inst_match.m
@@ -1780,9 +1780,10 @@ inst_is_ground_mt_2(ModuleInfo, MaybeType,
Inst, !Expansions) :-
         ),
         fail
     ;
-        ( Inst = not_reached
-        ; Inst = ground(_, _)
-        )
+        Inst = not_reached
+    ;
+        Inst = ground(_, HOInstInfo),
+        not ho_inst_info_is_nonstandard_func_mode(ModuleInfo, HOInstInfo)
     ;
         Inst = bound(_, InstResults, BoundInsts),
         inst_results_bound_inst_list_is_ground_mt_2(InstResults, BoundInsts,
@@ -1822,11 +1823,13 @@ inst_is_ground_or_any(ModuleInfo, Inst) :-
 inst_is_ground_or_any_2(ModuleInfo, Inst, !Expansions) :-
     require_complete_switch [Inst]
     (
-        ( Inst = ground(_, _)
-        ; Inst = any(_, _)
+        ( Inst = any(_, _)
         ; Inst = not_reached
         )
     ;
+        Inst = ground(_, HOInstInfo),
+        not ho_inst_info_is_nonstandard_func_mode(ModuleInfo, HOInstInfo)
+    ;
         Inst = bound(_, InstResults, BoundInsts),
         inst_results_bound_inst_list_is_ground_or_any_2(InstResults,
             BoundInsts, ModuleInfo, !Expansions)



More information about the reviews mailing list