[m-rev.] for review: fix bug #226 - incorrect purity of lazy.read_if_val/2

Paul Bone paul at bone.id.au
Tue Jan 8 12:10:26 AEDT 2013


On Tue, Jan 08, 2013 at 11:06:21AM +1100, Julien Fischer wrote:
> For review by Paul.
> 
> The comments in the bug report mentioned that we were awaiting some changes
> to the deep profiler before doing this, but since there is currently
> only a single
> call to read_if_val/2 in the deep profiler, I assume that these have been done.

No, that change never got made.  But I'm not sure that it is necessary (I've
forgotten some of the details over the last 12 months).  Your patch probably
makes it okay though.

> diff --git a/deep_profiler/autopar_types.m b/deep_profiler/autopar_types.m
> index 5d101b1..9565b6c 100644
> --- a/deep_profiler/autopar_types.m
> +++ b/deep_profiler/autopar_types.m
> @@ -310,14 +310,16 @@
> pard_goal_detail_annon_to_pard_goal_annon(SharedVarsSet, PGD, PG) :-
>      assoc_list(var_rep, float)::in, assoc_list(var_rep, float)::out) is det.
> 
>  build_var_use_list(Map, Var, !List) :-
> -    (
> -        map.search(Map, Var, LazyUse),
> -        read_if_val(LazyUse, Use)
> -    ->
> -        UseTime = Use ^ vui_cost_until_use,
> -        !:List = [Var - UseTime | !.List]
> -    ;
> -        true
> +    promise_pure (
> +        (
> +            map.search(Map, Var, LazyUse),
> +            impure read_if_val(LazyUse, Use)
> +        ->
> +            UseTime = Use ^ vui_cost_until_use,
> +            !:List = [Var - UseTime | !.List]
> +        ;
> +            true
> +        )
>      ).

I think this promise should be wider and the build_var_use_list should be
impure.  This is a change I can do later.  As it is your patch is better
than the previous version as it at least corrects the problem in lazy.m
Please commit it as is and open a new bug/issue for me to fix the promise.

-- 
Paul Bone
http://www.bone.id.au



More information about the reviews mailing list