[m-rev.] for review: fix bug #499 - switches on 64-bit integers in the Java grade

Peter Wang novalazy at gmail.com
Tue Feb 13 10:37:11 AEDT 2018


On Mon, 12 Feb 2018 11:43:23 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
> @@ -117,15 +118,17 @@ ml_simplify_switch(Stmt0, Stmt, !Info) :-
>           Stmt = Stmt0
>       ).
> 
> -:- func is_integral_type(mlds_type) = bool.
> +:- pred is_integral_type(mlds_type::in, int_type::out) is semidet.
> 
> -is_integral_type(MLDSType) = IsIntegral :-
> -    (
> +is_integral_type(MLDSType, IntType) :-
> +    require_complete_switch [MLDSType] (
>           ( MLDSType = mlds_native_int_type
> -        ; MLDSType = mlds_native_uint_type
>           ; MLDSType = mlds_native_char_type
>           ),
> -        IsIntegral = yes
> +        IntType = int_type_int

I suggest the usual formatting with the opening paren on a new line.

> diff --git a/compiler/switch_gen.m b/compiler/switch_gen.m
> index d919709..84a7ab3 100644
> --- a/compiler/switch_gen.m
> +++ b/compiler/switch_gen.m
> @@ -154,7 +155,11 @@ generate_switch(CodeModel, SwitchVar, CanFail, Cases, GoalInfo, Code,
>           MayUseSmartIndexing = may_use_smart_indexing,
>           module_info_get_globals(ModuleInfo, Globals),
>           (
> -            SwitchCategory = atomic_switch,
> +            % When generating we do not require any special treatment of
> +            % switches involving 64-bit integers.
> +            ( SwitchCategory = atomic_switch
> +            ; SwitchCategory = int64_switch
> +            ),
>               num_cons_ids_in_tagged_cases(TaggedCases, NumConsIds, NumArms),
>               ( if
>                   MaybeIntSwitchInfo =

It's not immediately clear what special information the comment is
trying to impart. Maybe just delete it.

Looks fine otherwise.

Peter


More information about the reviews mailing list