[m-dev.] for review: MLDS switch simplification

[Fergus Henderson]

On 21-Nov-2000, Peter Ross <peter.ross at miscrit.be> wrote:
> 
> Apart from the two minor issues this change looks fine.

Great, I'll go ahead and commit it then.  Thanks for the review.
Enclosed below is a relative diff that addresses your review comments.

> I would prefer this documentation to go above the pred declaration.

Done.

> > +	% Generate an rval which will be true iff the specified
> > +	% case condition matches the specified rval.
> > +:- func ml_gen_case_match_cond(mlds__case_match_cond, rval) = rval.
> > +ml_gen_case_match_cond(match_value(CaseRval), SwitchRval) =
> > +	% XXX is `eq' the right operator to use here?
> 
> I don't understand this XXX.
> What other operator could you be using?

The issue there is that `eq' assumes that the argument has integral
type.  It would be the wrong operator to use for strings or floats,
for example.

I've changed the documentation to make it clear that the relevant
routines only work for integral types, and deleted the XXX.

--- ml_simplify_switch.m.old	Tue Nov 21 21:38:03 2000
+++ ml_simplify_switch.m	Tue Nov 21 21:39:00 2000
@@ -116,17 +116,17 @@
 	Density = calc_density(NumCases, CasesRange),
 	Density > ReqDensity.
 
+	% For switches with a default, we normally need to check that
+	% the variable is in range before we index into the jump table.
+	% However, if the range of the type is sufficiently small,
+	% we can make the jump table large enough to hold all
+	% of the values for the type.
 :- pred maybe_eliminate_default(mlds__switch_range::in,
 		list(mlds__switch_case)::in, mlds__switch_default::in, int::in,
 		int::out, int::out, bool::out) is det.
 
 maybe_eliminate_default(Range, Cases, Default, ReqDensity,
 		FirstVal, LastVal, NeedRangeCheck) :-
-	% For switches with a default, we normally need to check that
-	% the variable is in range before we index into the jump table.
-	% However, if the range of the type is sufficiently small,
-	% we can make the jump table large enough to hold all
-	% of the values for the type.
 	(
 		Default \= default_is_unreachable,
 		Range = range(Min, Max),
@@ -411,7 +411,7 @@
 
 %-----------------------------------------------------------------------------%
 
-	% Convert a switch to a chain of if-then-elses
+	% Convert an int switch to a chain of if-then-elses
 	% that test each case in turn.
 	%
 :- func ml_switch_to_if_else_chain(mlds__switch_cases, mlds__switch_default,
@@ -445,7 +445,8 @@
 	).
 
 	% Generate an rval which will be true iff any of the specified
-	% list of case conditions matches the specified rval.
+	% list of case conditions matches the specified rval
+	% (which must have integral type).
 :- func ml_gen_case_match_conds(mlds__case_match_conds, rval) = rval.
 ml_gen_case_match_conds([], _) = const(false).
 ml_gen_case_match_conds([Cond], SwitchRval) =
@@ -456,10 +457,10 @@
 		ml_gen_case_match_conds([Cond2 | Conds], SwitchRval)).
 
 	% Generate an rval which will be true iff the specified
-	% case condition matches the specified rval.
+	% case condition matches the specified rval
+	% (which must have integral type).
 :- func ml_gen_case_match_cond(mlds__case_match_cond, rval) = rval.
 ml_gen_case_match_cond(match_value(CaseRval), SwitchRval) =
-	% XXX is `eq' the right operator to use here?
 	binop(eq, CaseRval, SwitchRval).
 ml_gen_case_match_cond(match_range(MinRval, MaxRval), SwitchRval) =
 	binop(and, binop(>=, SwitchRval, MinRval),

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------