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

Bug in handling the JSON unboxed type when switching on it #6789

Closed
jfrolich opened this issue May 28, 2024 · 3 comments · Fixed by #6792
Closed

Bug in handling the JSON unboxed type when switching on it #6789

jfrolich opened this issue May 28, 2024 · 3 comments · Fixed by #6792
Labels

Comments

@jfrolich
Copy link

Repro here

@zth
Copy link
Collaborator

zth commented May 29, 2024

cc @cristianoc

@cristianoc
Copy link
Collaborator

Smaller self contained repro:

type obj = {name:string}

@unboxed
type t =
  | Boolean(bool)
  | Object(obj)
  | Array(array<int>)

switch Object({name: "hello"}) {
| Object(v) => Console.log(v)
| Array(v) => Console.log(v)
| _ => Console.log("Not an object or array1")
}

produces:

var v = {
  name: "hello"
};

if (Array.isArray(v)) {
  console.log(v);
} else {
  switch (typeof v) {
    case "boolean" :
        console.log("Not an object or array1");
        break;
    case "object" :
    
  }
}

It seems the code for case "object" is not emitted.

@cristianoc
Copy link
Collaborator

cristianoc commented May 29, 2024

The issue happens when 2 cases, here Object and Array, have identical body (here Console.log(v)).
The switch-generation code, which was not designed with untagged unions in mind, merges the two cases into one (and makes one of the two empty).

However, for Object and Array, what's generated is not a straight switch, but a mix of if-then-else and switch. This means that the Object and Array cases are apart in the generated code, and merging them (making one empty) is wrong.

cristianoc added a commit that referenced this issue May 29, 2024
… object and array.

Fixes #6789

The issue happens when 2 cases, here `Object` and `Array`, have identical body (here `Console.log(v)`).
The switch-generation code, which was not designed with untagged unions in mind, merges the two cases into one (and makes one of the two empty).

However, for `Object` and `Array`, what's generated is not a straight switch, but a mix of `if-then-else` and `switch`. This means that the `Object` and `Array` cases are apart in the generated code, and merging them (making one empty) is wrong.
cristianoc added a commit that referenced this issue May 29, 2024
… object and array.

Fixes #6789

The issue happens when 2 cases, here `Object` and `Array`, have identical body (here `Console.log(v)`).
The switch-generation code, which was not designed with untagged unions in mind, merges the two cases into one (and makes one of the two empty).

However, for `Object` and `Array`, what's generated is not a straight switch, but a mix of `if-then-else` and `switch`. This means that the `Object` and `Array` cases are apart in the generated code, and merging them (making one empty) is wrong.
cristianoc added a commit that referenced this issue May 30, 2024
… object and array.

Fixes #6789

The issue happens when 2 cases, here `Object` and `Array`, have identical body (here `Console.log(v)`).
The switch-generation code, which was not designed with untagged unions in mind, merges the two cases into one (and makes one of the two empty).

However, for `Object` and `Array`, what's generated is not a straight switch, but a mix of `if-then-else` and `switch`. This means that the `Object` and `Array` cases are apart in the generated code, and merging them (making one empty) is wrong.
cknitt pushed a commit to cknitt/rescript that referenced this issue May 30, 2024
… object and array.

Fixes rescript-lang#6789

The issue happens when 2 cases, here `Object` and `Array`, have identical body (here `Console.log(v)`).
The switch-generation code, which was not designed with untagged unions in mind, merges the two cases into one (and makes one of the two empty).

However, for `Object` and `Array`, what's generated is not a straight switch, but a mix of `if-then-else` and `switch`. This means that the `Object` and `Array` cases are apart in the generated code, and merging them (making one empty) is wrong.

# Conflicts:
#	CHANGELOG.md
#	jscomp/test/UntaggedVariants.js
cknitt pushed a commit that referenced this issue May 30, 2024
… object and array.

Fixes #6789

The issue happens when 2 cases, here `Object` and `Array`, have identical body (here `Console.log(v)`).
The switch-generation code, which was not designed with untagged unions in mind, merges the two cases into one (and makes one of the two empty).

However, for `Object` and `Array`, what's generated is not a straight switch, but a mix of `if-then-else` and `switch`. This means that the `Object` and `Array` cases are apart in the generated code, and merging them (making one empty) is wrong.

# Conflicts:
#	CHANGELOG.md
#	jscomp/test/UntaggedVariants.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants