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

not merged equal pattern matching branches #6359

Closed
vicuna opened this Issue Apr 1, 2014 · 3 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

vicuna commented Apr 1, 2014

Original bug ID: 6359
Reporter: @chambart
Assigned to: @maranget
Status: closed (set by @xavierleroy on 2015-12-11T18:26:28Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.02.0+dev
Category: back end (clambda to assembly)
Monitored by: @gasche

Bug description

In the following example (extracted from ocamlnet), the fields extractions of n and g for cases A and D are not merged.

match C with | A(n,g)
| `D(n,g) -> n + g
| _ -> 0

In the resulting lambda

ocamlc -c -drawlambda example.ml

(setglobal Example!
(seq
(let (match/1012 67a)
(catch
(catch
(if (isint match/1012) (exit 2)
(let (variant/1019 (field 0 match/1012))
(if (!= variant/1019 65)
(if (!= variant/1019 68) (exit 2)
(let
(match/1017 (field 1 match/1012)
g/1016 (field 1 match/1017)
n/1015 (field 0 match/1017))
(exit 1 n/1015 g/1016)))
(let
(match/1017 (field 1 match/1012)
g/1016 (field 1 match/1017)
n/1015 (field 0 match/1017))
(exit 1 n/1015 g/1016)))))
with (2) 0)
with (1 n/1008 g/1009) (+ n/1008 g/1009)))
(makeblock 0)))

This is not strictly speaking a bug since it does not impact anything,
but I would like to assume that identifiers can't be bound multiple times
(in my backend patches) and this is the only case I know (in the opam
repository) where this don't hold.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Apr 2, 2014

Comment author: @garrigue

Multiple binding of the same identifier is a bug: there are already parts of the backend where this invariant is assumed, and I have generated wrong code by just breaking it.
Whether to merge the branches or not is another question.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Apr 2, 2014

Comment author: @maranget

This is indeed a bug, and it is related to sharing!
(The match compiler re-shares actions which are alpha-equivalent
but this sharing is lost in the case for instance of variants.

--Luc

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Apr 2, 2014

Comment author: @maranget

Bug fixed in trunk. Thanks to all.

--Luc

@vicuna vicuna closed this Dec 11, 2015

@vicuna vicuna added the back-end label Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

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.