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

< is weirdly sensitive to being less-than or extends in SAFE #1225

Closed
bbrk24 opened this issue May 6, 2024 · 4 comments · Fixed by #1447
Closed

< is weirdly sensitive to being less-than or extends in SAFE #1225

bbrk24 opened this issue May 6, 2024 · 4 comments · Fixed by #1447

Comments

@bbrk24
Copy link
Contributor

bbrk24 commented May 6, 2024

These two functions are not compiled alike:

x := &: number < 0 ? -1 : 1
y := &: number < 0 ? -1 : &
const x = ($: number extends 0 ? -1 : 1) => $;
const y = ($1: number) => ($1 < 0 ? -1 : $1);

Even more strangely, if you swap them (defining y before x), that's a ParseError.

@edemaine
Copy link
Collaborator

edemaine commented May 6, 2024

I'm not sure what the issue is exactly. The first example is a valid type, so is treated as such, while the second isn't, so the longest possible prefix that's a type gets used. In general, Civet (Hera/PEG) greedily matches what it can for a given rule.

What was the behavior you expected? Are you suggesting we e.g. disable < as extends shorthand in &, maybe without parens?

@bbrk24
Copy link
Contributor Author

bbrk24 commented May 6, 2024

One issue is when one of the branches is a call:

x := &: number < 0 ? -1 : Math.sqrt 5
y := &: number < 0 ? Math.sqrt 5 : 1
const x = ($: number extends 0 ? -1 : Math.sqrt) => $(5);
const y = ($1: number) => ($1 < 0 ? Math.sqrt(5) : 1);

Really this just caught me off-guard: I forgot < could also mean extends, and I had to look at the output to understand the slew of errors TS gave me. This is pretty trivially worked around by wrapping &: number in parens.

@edemaine
Copy link
Collaborator

edemaine commented May 6, 2024

Another option that comes to mind is requiring a typed & to use parentheses, as in (&: number). The current syntax is partly a holdover from when & couldn't be lifted outside parentheses like it can now. Of course, now that we have a parenless version, it's tempting to have it... After all, the point of & is to be more concise than an arrow function.

@bbrk24
Copy link
Contributor Author

bbrk24 commented May 6, 2024

We could allow the parenless version for simple types like number, but require parens for conditional types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants