Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Array.of_seq #1897

Merged
merged 2 commits into from Jul 11, 2018

Conversation

Projects
None yet
4 participants
@thierry-martinez
Copy link
Contributor

commented Jul 11, 2018

Reported at https://caml.inria.fr/mantis/view.php?id=7820

Array.of_seq applies a circular permutation of one cell to the right
on the sequence.

With OCaml 4.07.0 and trunk, we have

# Array.of_seq (Array.to_seq [| 1; 2; 3 |]);;
- : int array = [|3; 1; 2|]

In stdlib/array.ml, line 337 (last line of of_rev_list), we have

fill (len-1) tl

whereas it should be

fill (len-2) tl

since hd, which should be assigned to the cell len - 1, is skipped.

Fix Array.of_seq
Reported at https://caml.inria.fr/mantis/view.php?id=7820

Array.of_seq applies a circular permutation of one cell to the right
on the sequence.

With OCaml 4.07.0 and trunk, we have

- : int array = [|3; 1; 2|]

In stdlib/array.ml, line 337 (last line of of_rev_list), we have
      fill (len-1) tl
whereas it should be
      fill (len-2) tl
since hd, which should be assigned to the cell (len - 1), is skipped.
@nojb

nojb approved these changes Jul 11, 2018

Copy link
Contributor

left a comment

LGTM! Could you please add a Changes entry? Thanks!

@xavierleroy

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2018

Thanks for spotting and fixing this bug. Merging right now.

@xavierleroy xavierleroy merged commit 220063a into ocaml:trunk Jul 11, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

xavierleroy added a commit that referenced this pull request Jul 11, 2018

Fix Array.of_seq (#1897)
Reported at https://caml.inria.fr/mantis/view.php?id=7820

Array.of_seq applies a circular permutation of one cell to the right
on the sequence.

With OCaml 4.07.0 and trunk, we have

- : int array = [|3; 1; 2|]

In stdlib/array.ml, line 337 (last line of of_rev_list), we have
      fill (len-1) tl
whereas it should be
      fill (len-2) tl
since hd, which should be assigned to the cell (len - 1), is skipped.
@xavierleroy

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2018

Cherry-picked to 4.07 branch, commit b5ff016

@gasche

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

This sounds like 4.07.1 territory to me. I'll create a Changes entry section for a potential future 4.07 release and move it there.

@xavierleroy

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2018

I wasn't quite sure how to reorganize the Changes file on trunk, so I didn't touch it, only the Changes on branch 4.07. Thanks for doing it properly.

thierry-martinez added a commit to thierry-martinez/opam-repository that referenced this pull request Sep 10, 2018

stdcompat-6
- Support for OCaml 4.08.0

- Support VPATH build, i.e. when configure is executed in a build directory
  distinct from the source directory

- Lexing.new_line

- Bigarray: only available from 4.02.0. From 4.02.0 and before 4.07.0,
  --no-alias-deps is used to allow the alias to appear in Stdcompat without
  requiring the library to be linked. Bigarray library should be linked if it is
  effectively used.

- Fix implementations with --disable-magic

- Fix license conflict: The project was intended to be under BSD license
  since the beginning, and a LICENSE file was provided accordingly.
  However, automake generated a COPYING file with the GPLv3 by default. The
  COPYING file now contains the BSD license, and the LICENSE file is removed.
  (reported by Török Edwin,
   thierry-martinez/stdcompat#5)

- Fix auto-generated interfaces for Hashtbl.MakeSeeded

- Exceptions are reexported again. They were missing in auto-generated
  interfaces.

- min_binding_opt and max_binding_opt are not redefined if OCaml >=4.05.

- Array.of_seq is redefined in OCaml 4.07.0 to circumvent a bug in the
  implementation of the standard library. See:
    - https://caml.inria.fr/mantis/view.php?id=7820
    - ocaml/ocaml#1897

@thierry-martinez thierry-martinez referenced this pull request Sep 10, 2018

Merged

stdcompat-6 #12596

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.