Skip to content

ReScript v12 - pattern matching on optional fields breaks other cases #7400

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

Closed
DZakh opened this issue Apr 21, 2025 · 3 comments · Fixed by #7440
Closed

ReScript v12 - pattern matching on optional fields breaks other cases #7400

DZakh opened this issue Apr 21, 2025 · 3 comments · Fixed by #7440
Labels
Milestone

Comments

@DZakh
Copy link
Member

DZakh commented Apr 21, 2025

https://rescript-lang.org/try?version=v12.0.0-alpha.10&module=esmodule&code=C4TwDgpgBMCGDmUC8UBCB7dAbCsB2UAPlAHICuAtgEYQBOAUKJFAGbq0WzDJQCSewAMwAmeo3DQAzgGMAFhE48A3vShRp6PJOAB+AFxQqmLABpVrdp10G2HLmbVx4Bp2YC+YnN1oQWASzxoFBl5RSQAPigVNUkAdz9gOSgQhVgo82IlDS1gAwB9N2RIgCJijKjbKwN+IWFCiKhigNqytUynAwxsXDx6kqNu-FaiKDyixoATX1gyLGBhjw8gA

v12 (broken):

function refine(schema) {
  if (schema.const !== undefined) {
    return "";
  } else {
    return "int32";
  }
}

v11 (working):

function refine(schema) {
  if (schema.const !== undefined) {
    return "";
  } else if (schema.format !== undefined) {
    return "int32";
  } else if (schema.tag === "Boolean") {
    return "boolean";
  } else {
    return "default";
  }
}
@DZakh
Copy link
Member Author

DZakh commented Apr 21, 2025

Broken between alpha.4 and alpha.5

@cristianoc
Copy link
Collaborator

This seems to happen when variant format has only one case, and that single case Int32 is used as a case in the pattern matching.

type tag = Boolean | Number
type format = Int32

type schema = {
  const?: bool,
  format?: format,
  tag: tag,
}

let refine = schema => {
  switch schema {
  | {const: _} => ""
  | {format: Int32} => "int32"
  | {tag: Boolean} => "boolean"
  | _ => "default"
  }
}

@cristianoc
Copy link
Collaborator

Here's a simplified example, showing that using the explicit single value triggers the issue:

type format = Int32

type schema = {
  format?: format,
}

let bad = schema => {
  switch schema {
  | {format: Int32} => "int32"
  | _ => "default"
  }
}

let good = schema => {
  switch schema {
  | {format: _} => "int32"
  | _ => "default"
  }
}

compiles to:

function bad(schema) {
  return "int32";
}

function good(schema) {
  if (schema.format !== undefined) {
    return "int32";
  } else {
    return "default";
  }
}

@cristianoc cristianoc moved this to Backlog in ReScript development May 3, 2025
@cknitt cknitt added this to the v12 milestone May 4, 2025
@cknitt cknitt added the bug label May 4, 2025
cristianoc added a commit that referenced this issue May 8, 2025
Fixes #7400

Fix regression in pattern  matching for optional fields.

There's an optimisation where `{field: VariantCase}` is compiled to `field:VariantCase` instead of `field:Some(VariantCase)` to avoid generating code that handles undefined, since `VariantCase` is going to be a string.
However, that optimisation happens early in the pattern matching compiler, and can end up interfering with other cases. For example, when `VariantCase` is the only case in a variant, and the following case is `_`, the compiler is induced in thinking that the variant case covers all cases, and the case for `_` is never generated.
Similar behaviour happens when handling the last case of a variant followed by `_`.
cristianoc added a commit that referenced this issue May 8, 2025
Fixes #7400

Fix regression in pattern  matching for optional fields.

There's an optimisation where `{field: VariantCase}` is compiled to `field:VariantCase` instead of `field:Some(VariantCase)` to avoid generating code that handles undefined, since `VariantCase` is going to be a string.
However, that optimisation happens early in the pattern matching compiler, and can end up interfering with other cases. For example, when `VariantCase` is the only case in a variant, and the following case is `_`, the compiler is induced in thinking that the variant case covers all cases, and the case for `_` is never generated.
Similar behaviour happens when handling the last case of a variant followed by `_`.
cristianoc added a commit that referenced this issue May 8, 2025
Fixes #7400

Fix regression in pattern  matching for optional fields.

There's an optimisation where `{field: VariantCase}` is compiled to `field:VariantCase` instead of `field:Some(VariantCase)` to avoid generating code that handles undefined, since `VariantCase` is going to be a string.
However, that optimisation happens early in the pattern matching compiler, and can end up interfering with other cases. For example, when `VariantCase` is the only case in a variant, and the following case is `_`, the compiler is induced in thinking that the variant case covers all cases, and the case for `_` is never generated.
Similar behaviour happens when handling the last case of a variant followed by `_`.
@github-project-automation github-project-automation bot moved this from Backlog to Done in ReScript development May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants