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

Jump to bottom

Odd behaviour of or-patterns with extensible typ #7661

Closed
vicuna opened this issue Oct 26, 2017 · 14 comments
Closed

Odd behaviour of or-patterns with extensible typ #7661

vicuna opened this issue Oct 26, 2017 · 14 comments
Assignees
@maranget
Labels
bug middle-end

Comments

@vicuna
Copy link

vicuna commented Oct 26, 2017

Original bug ID: 7661
Reporter: @thizanne
Assigned to: @maranget
Status: resolved (set by @trefis on 2018-01-09T11:56:53Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 4.05.0
Fixed in version: 4.06.1+dev/rc1/rc2
Category: middle end (typedtree to clambda)
Related to: #5788
Monitored by: @gasche

Bug description

On this example, the evaluation of f A B doesn't go in the (A | B) , B branch. It works as intended if t is a non-extensible sum type. If the two first lines of the pattern matching are inverted, then a has the unexpected '_' value.

type t = ..
type t +=
  | A
  | B

let f x y = match x, y with
  | (A | B), A -> 'a'
  | (A | B), B -> 'b'
  | _, _ -> '_'

let a = f A A (* a : char = 'a' *)
let b = f A B (* b : char = '_' *)
@vicuna
Copy link
Author

vicuna commented Oct 26, 2017

Comment author: @thizanne

Original report by Abdelraouf Ouadjaout.

@vicuna
Copy link
Author

vicuna commented Oct 26, 2017

Comment author: @gasche

By "odd behavior" you mean "major bug" ! Thanks for the report.

@vicuna
Copy link
Author

vicuna commented Oct 26, 2017

Comment author: @gasche

I could reproduce the bug with all versions of OCaml that support extensible datatypes, from 4.02.1 to 4.06.0+beta2.

@vicuna
Copy link
Author

vicuna commented Oct 26, 2017

Comment author: @alainfrisch

Could reproduce with exceptions as well, of course.

@vicuna
Copy link
Author

vicuna commented Oct 26, 2017

Comment author: @yallop

The bug appears to have been introduced in this commit, first released in 4.02.0:

32bcc18

@vicuna
Copy link
Author

vicuna commented Oct 26, 2017

Comment author: @gasche

I pinged Luc (and Jacques) so hopefully they will look at it. Incidentially, I'm meeting Thomas and Luc tomorrow to discuss pattern-matching things, so I guess that must be put on our list. (Of course anyone with a fix before that is warmly welcome.)

@vicuna
Copy link
Author

vicuna commented Oct 27, 2017

Comment author: @gasche

To reproduce with exceptions, just replace

type t = .. type t += A | B

with

exception A exception B

The correct lambda-code, from the state just before the faulty commit yallop found, looks like this:

(setglobal Repro!
(let
(A/1008 (makeblock 0 "Repro.A")
B/1009 (makeblock 0 "Repro.B")
f/1010
(function x/1011 y/1012
(catch
(catch
(if (== (field 0 x/1011) A/1008) (exit 5)
(if (== (field 0 x/1011) B/1009) (exit 5) (exit 4)))
with (5)
(if (== (field 0 y/1012) A/1008) 'a'
(if (== (field 0 y/1012) B/1009) 'b' (exit 4))))
with (4) '_'))
a/1013 (apply f/1010 (makeblock 0 A/1008) (makeblock 0 A/1008))
b/1014 (apply f/1010 (makeblock 0 A/1008) (makeblock 0 B/1009))
match/1020
(seq (apply (field 25 (global Pervasives!)) b/1014)
(apply (field 30 (global Pervasives!)) 0a)))
(makeblock 0 A/1008 B/1009 f/1010 a/1013 b/1014)))

The incorrect lambda-code (after this commit) looks like

(setglobal Repro!
(seq (opaque (global Pervasives!))
(let
(A/1199 = (makeblock 248 "Repro.A" (caml_fresh_oo_id 0))
B/1200 = (makeblock 248 "Repro.B" (caml_fresh_oo_id 0))
f/1201 =
(function x/1202 y/1203
(catch
(catch
(catch
(if (== x/1202 A/1199) (exit 7)
(if (== x/1202 B/1200) (exit 7)
(if (== x/1202 A/1199) (exit 8)
(if (== x/1202 B/1200) (exit 8) (exit 6)))))
with (7) (if (== y/1203 A/1199) 'a' (exit 6)))
with (8) (if (== y/1203 B/1200) 'b' (exit 6)))
with (6) '_'))
a/1204 = (apply f/1201 A/1199 A/1199)
b/1205 = (apply f/1201 A/1199 B/1200)
match/1214 =
(seq (apply (field 25 (global Pervasives!)) b/1205)
(apply (field 31 (global Pervasives!)) 0a)))
(makeblock 0 A/1199 B/1200 f/1201 a/1204 b/1205))))

@vicuna
Copy link
Author

vicuna commented Oct 31, 2017

Comment author: @yallop

Luc has implemented a fix: #1459

@vicuna
Copy link
Author

vicuna commented Nov 22, 2017

Comment author: @gasche

Fixed in trunk and the 4.06 maintenance branch.

Thanks thizanne for the report! May I recommend that you find your next major bug during the development or beta-testing period, rather than during the release-candidate period? :-)

@vicuna
Copy link
Author

vicuna commented Nov 27, 2017

Comment author: @hhugo

Compiling jbuilder with trunk gives:

Fatal error: exception Invalid_argument("index out of bounds")
Raised by primitive operation at file "bytecomp/matching.ml", line 1582, characters 34-59
Called from file "list.ml", line 100, characters 12-15
Called from file "bytecomp/matching.ml", line 1582, characters 2-73
Called from file "bytecomp/matching.ml", line 1589, characters 4-48
Called from file "bytecomp/matching.ml", line 155, characters 16-28
Called from file "bytecomp/matching.ml", line 176, characters 14-39
Called from file "bytecomp/matching.ml", line 1618, characters 15-56
Called from file "bytecomp/matching.ml", line 1175, characters 25-39
Called from file "bytecomp/matching.ml", line 1179, characters 8-27
Called from file "bytecomp/matching.ml", line 2756, characters 40-59
Called from file "bytecomp/matching.ml", line 2683, characters 6-145
Called from file "bytecomp/matching.ml", line 2501, characters 36-64
Called from file "bytecomp/matching.ml", line 2543, characters 14-47
Called from file "bytecomp/matching.ml", line 2620, characters 20-131
Called from file "bytecomp/matching.ml", line 2623, characters 18-151
Called from file "bytecomp/matching.ml", line 2634, characters 6-31
Called from file "bytecomp/matching.ml", line 2683, characters 6-145
Called from file "bytecomp/matching.ml", line 2879, characters 28-71
Called from file "bytecomp/translcore.ml", line 700, characters 59-76
Called from file "bytecomp/translcore.ml", line 1074, characters 30-46
Called from file "bytecomp/translcore.ml", line 1081, characters 9-35
Called from file "list.ml", line 82, characters 20-23
Called from file "bytecomp/translcore.ml", line 1204, characters 9-29
Called from file "bytecomp/translcore.ml", line 1181, characters 8-68
Called from file "bytecomp/translcore.ml", line 1181, characters 8-68
Called from file "bytecomp/translcore.ml", line 1181, characters 8-68
Called from file "bytecomp/translcore.ml", line 703, characters 8-213
Called from file "bytecomp/translobj.ml", line 182, characters 17-20
Re-raised at file "bytecomp/translobj.ml", line 199, characters 4-13
Called from file "bytecomp/translcore.ml", line 1213, characters 20-35
Called from file "bytecomp/translmod.ml", line 519, characters 10-48
Called from file "bytecomp/translmod.ml", line 517, characters 12-69
Called from file "bytecomp/translmod.ml", line 517, characters 12-69
Called from file "bytecomp/translmod.ml", line 517, characters 12-69
Called from file "bytecomp/translmod.ml", line 435, characters 14-52
Called from file "bytecomp/translmod.ml", line 904, characters 16-74
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 871, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 915, characters 27-78
Called from file "bytecomp/translmod.ml", line 898, characters 37-188
Called from file "bytecomp/translmod.ml", line 871, characters 37-188
Called from file "bytecomp/translmod.ml", line 871, characters 37-188
Called from file "bytecomp/translmod.ml", line 871, characters 37-188
Called from file "bytecomp/translmod.ml", line 1031, characters 21-78
Called from file "bytecomp/translobj.ml", line 138, characters 13-18
Called from file "bytecomp/translmod.ml", line 1097, characters 18-65
Called from file "utils/misc.ml", line 28, characters 20-27
Re-raised at file "utils/misc.ml", line 28, characters 50-57
Called from file "driver/optcompile.ml" (inlined), line 67, characters 15-18
Called from file "driver/optcompile.ml", line 121, characters 10-133
Called from file "driver/optcompile.ml", line 139, characters 8-68
Re-raised at file "driver/optcompile.ml", line 144, characters 6-13
Called from file "utils/misc.ml", line 28, characters 20-27
Re-raised at file "utils/misc.ml", line 28, characters 50-57
Called from file "driver/compenv.ml", line 576, characters 6-35
Called from file "list.ml", line 100, characters 12-15
Called from file "driver/compenv.ml", line 652, characters 2-61
Called from file "driver/optmain.ml", line 253, characters 6-163
Re-raised at file "parsing/location.ml", line 465, characters 14-25
Re-raised at file "parsing/location.ml", line 465, characters 14-25
Re-raised at file "parsing/location.ml", line 465, characters 14-25
Re-raised at file "parsing/location.ml", line 465, characters 14-25
Re-raised at file "parsing/location.ml", line 465, characters 14-25
Re-raised at file "parsing/location.ml", line 465, characters 14-25
Called from file "parsing/location.ml" (inlined), line 470, characters 31-61
Called from file "driver/optmain.ml", line 313, characters 6-37
Called from file "driver/optmain.ml", line 317, characters 2-9

@vicuna
Copy link
Author

vicuna commented Nov 27, 2017

Comment author: @mshinwell

Re-assigned to Luc for fixing.

@vicuna
Copy link
Author

vicuna commented Nov 27, 2017

Comment author: @hhugo

instructions to reproduce the issue:

opam switch 4.07.0+trunk
opam install jbuilder

@vicuna
Copy link
Author

vicuna commented Nov 28, 2017

Comment author: @maranget

Ok, I have reproduced the bug, then reduced it, and then, hopefully, fixed it.
Please see pull request 1493 on github.

#1493

@vicuna
Copy link
Author

vicuna commented Jan 2, 2018

Comment author: @trefis

Since #1538 was merged I think we can mark this as resolved again.

@vicuna vicuna closed this as completed Jan 9, 2018
@vicuna vicuna mentioned this issue Oct 27, 2017
Closed
@vicuna vicuna added the bug label Mar 20, 2019
@gasche gasche mentioned this issue Mar 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maranget maranget

Labels
bug middle-end
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@maranget @vicuna