[m-rev.] for post-commit review: fix mantis bug #367

Zoltan Somogyi zoltan.somogyi at runbox.com
Sat Dec 6 19:43:18 AEDT 2014



On Sat, 6 Dec 2014 12:34:49 +1100, Peter Wang <novalazy at gmail.com> wrote:
> > Or are you worried about the compiler moving Mid < Hi to AFTER
> > the lookup? I am pretty sure the compiler is not allowed to do that,
> 
> Yes, that is the worry.  The "strict commutative" operational semantics
> allows it, does it not?

I haven't checked lately, but it definitely shouldn't. The compiler should
not be allowed to turn "fail, infinite_loop" into "infinite_loop, fail", so it
should not be able to move computations before a point of failure.

However, I agree that we can remove the very slight risk that it might
by turning the conjunction into an if-then-else, whose cond and then
parts are not allowed to be reordered in any semantics. Which is why
I am committing this diff.

--- a/library/array.m
+++ b/library/array.m
@@ -2209,12 +2209,12 @@ approx_binary_search_loop(Cmp, A, X, Lo, Hi, I) :-
     ;
         O = (<),
         ( if
-            (
-                Mid < Hi,
-                % Mid + 1 cannot exceed Hi, so the array access is safe.
+            ( if Mid < Hi then
+                % We get here only if Mid + 1 cannot exceed Hi,
+                % so the array access is safe.
                 array.unsafe_lookup(A, Mid + 1, MidP1X),
                 (<) = Cmp(X, MidP1X)
-            ;
+            else
                 Mid = Hi
             )
         then


Zoltan.





More information about the reviews mailing list