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

Prevent fatal compiler error with unboxing code and GADTs #1578

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

Ekdohibs
Copy link
Contributor

This prevents a crash on this example:

type foo =
  | A of float
  | B of int

type bar = Other

type 'a t =
  | Foo : foo -> foo t
  | Bar : bar t


let f (type a) : a t -> a = function
  | Foo x -> x
  | Bar -> Other


let baz p x z =
  let a =
    if p then (f x)
    else B z
  in
  Sys.opaque_identity a

I think we could also rewrite the code to Invalid in this case, but I'm not sure how to do that.

@Ekdohibs Ekdohibs requested review from chambart and Gbury as code owners July 19, 2023 17:03
@mshinwell mshinwell added bug Something isn't working flambda2 Prerequisite for, or part of, flambda2 labels Jul 24, 2023
@mshinwell mshinwell changed the title Prevent crash with unboxing code and GADTs Prevent fatal compiler error with unboxing code and GADTs Jul 24, 2023
Copy link
Contributor

@chambart chambart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to think about potential problems since this might be a bit tricky. But this seems sound.

It would be possible to produce invalid in that case, but it is probably too much work for quite a small benefit.

@Ekdohibs Ekdohibs requested a review from lukemaurer as a code owner July 25, 2023 09:13
@mshinwell mshinwell merged commit 83fa0b2 into ocaml-flambda:main Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flambda2 Prerequisite for, or part of, flambda2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants