[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