Fix an abort when constructing MLDS lookup tables.

This fixes Mantis bug #481.

compiler/ml_code_util.m:
    We used to approach the construction of tables for lookup switches
    by testing whether the shape of the HLDS code fit our requirements,
    and then building those tables using code that assumed that all the
    variables involved represented constants. This approach had a bug:
    when a switch arm constructed an output using *only* the switched-on
    variable, this passed the shape test even when that variable wasn't
    a constant.

    We could fix the shape test, instead this diff changes the approach
    to avoid making the incorrect assumption. This seems more robust,
    and in any case it is the approach used by the LLDS backend.

compiler/ml_disj_gen.m:
    Make the same switch in approach when generating lookup tables
    for disjunctions.

tests/valid/bug481.m:
    The Mantis test case.

tests/valid/Mmakefile:
    Enable the new test case.

compiler/ml_code_util.m

@@ -423,10 +423,10 @@
 %
 
 :- pred ml_generate_constants_for_arms(list(prog_var)::in, list(hlds_goal)::in,
-    list(list(mlds_rval))::out, ml_gen_info::in, ml_gen_info::out) is det.
+    list(list(mlds_rval))::out, ml_gen_info::in, ml_gen_info::out) is semidet.
 
 :- pred ml_generate_constants_for_arm(list(prog_var)::in, hlds_goal::in,
-    list(mlds_rval)::out, ml_gen_info::in, ml_gen_info::out) is det.
+    list(mlds_rval)::out, ml_gen_info::in, ml_gen_info::out) is semidet.
 
 :- pred ml_generate_field_assign(mlds_lval::in, mlds_type::in,
     mlds_field_id::in, mlds_vector_common::in, mlds_type::in,
@@ -1656,24 +1656,14 @@ ml_generate_constants_for_arm(Vars, Goal, Soln, !Info) :-
     ml_gen_info_get_const_var_map(!.Info, InitConstVarMap),
     ml_gen_goal(model_det, Goal, _LocalVarDefns, _FuncDefns, _Stmts, !Info),
     ml_gen_info_get_const_var_map(!.Info, FinalConstVarMap),
-    list.map(lookup_ground_rval(FinalConstVarMap), Vars, Soln),
+    list.map(search_ground_rval(FinalConstVarMap), Vars, Soln),
     ml_gen_info_set_const_var_map(InitConstVarMap, !Info).
 
-:- pred lookup_ground_rval(ml_ground_term_map::in, prog_var::in,
-    mlds_rval::out) is det.
+:- pred search_ground_rval(ml_ground_term_map::in, prog_var::in,
+    mlds_rval::out) is semidet.
 
-lookup_ground_rval(FinalConstVarMap, Var, Rval) :-
-    % We can do a map.lookup instead of a map.search here because
-    % - we execute this code only if we have already determined that
-    %   goal_is_conj_of_unify succeeds for this arm,
-    % - we don't even start looking for lookup switches unless we know
-    %   that the mark_static_terms pass has been run, and
-    % - for every arm on which goal_is_conj_of_unify succeeds,
-    %   mark_static_terms will mark all the variables to which Var
-    %   may be bound as being constructed statically. (There can be no need
-    %   to construct them dynamically, since all the arm's nonlocals are
-    %   output, which means none of them can be input.)
-    map.lookup(FinalConstVarMap, Var, GroundTerm),
+search_ground_rval(FinalConstVarMap, Var, Rval) :-
+    map.search(FinalConstVarMap, Var, GroundTerm),
     GroundTerm = ml_ground_term(Rval, _, _).
 
 ml_generate_field_assign(OutVarLval, FieldType, FieldId, VectorCommon,

compiler/ml_disj_gen.m

@@ -172,8 +172,7 @@ ml_gen_disj(Disjuncts, GoalInfo, CodeModel, Context, Stmts, !Info) :-
                 StaticGroundCells = yes,
 
                 DisjNonLocals = goal_info_get_nonlocals(GoalInfo),
-                all_disjuncts_are_conj_of_unify(DisjNonLocals, Disjuncts)
-            then
+                all_disjuncts_are_conj_of_unify(DisjNonLocals, Disjuncts),
                 % Since the MLDS backend implements trailing by a
                 % HLDS-to-HLDS transform (which is in add_trail_ops.m),
                 % if we get here, then trailing is not enabled, and we do
@@ -182,7 +181,8 @@ ml_gen_disj(Disjuncts, GoalInfo, CodeModel, Context, Stmts, !Info) :-
                 NonLocals = goal_info_get_nonlocals(GoalInfo),
                 OutVars = set_of_var.to_sorted_list(NonLocals),
                 list.map_foldl(ml_generate_constants_for_arm(OutVars),
-                    Disjuncts, Solns, !Info),
+                    Disjuncts, Solns, !Info)
+            then
                 ml_gen_lookup_disj(OutVars, Solns, Context, Stmts, !Info)
             else
                 ml_gen_ordinary_model_non_disj(FirstDisjunct, LaterDisjuncts,

tests/valid/Mmakefile

@@ -87,6 +87,7 @@ OTHER_PROGS = \
 	bug457 \
 	bug480 \
 	bug480a \
+	bug481 \
 	bug51 \
 	bug85 \
 	builtin_false \

tests/valid/bug481.m

@@ -0,0 +1,38 @@
+%---------------------------------------------------------------------------%
+% vim: ft=mercury ts=4 sw=4 et
+%---------------------------------------------------------------------------%
+%
+% Before 2019 aug 19, the compiler aborted when t compiling this in hlc.gc.
+% The problem was a map.lookup that tried to look up the variable String's
+% entry in the constant variable map. Since String is not a constant,
+% the lookup aborted. The MLDS code generator had code to ensure that
+% the switch (or disjunction) arms on which it tried to do this consisted
+% only of conjunctions of unifications which had only its outputs as its
+% nonlocal variables, but this code had a bug, which allowed the attempt
+% to generate a lookup table to proceed if, beside its outputs, an arm
+% also had the switched-on variable as a nonlocal. The second arm of the
+% switch on String below uses String in exactly such a manner, triggering
+% the bug.
+
+:- module bug481.
+
+:- interface.
+
+:- type result
+    --->    none
+    ;       enabled(string).
+
+:- pred parse(string::in, result::out) is semidet.
+
+:- implementation.
+
+parse(String, Res) :-
+    (
+        String = "none",
+        Res = none
+    ;
+        ( String = "enable"
+        ; String = "full"
+        ),
+        Res = enabled(String)
+    ).