Skip to content

Conversation

cknitt
Copy link
Member

@cknitt cknitt commented Oct 14, 2025

Fixes #7960

@cknitt cknitt requested review from cometkim and cristianoc October 14, 2025 05:59
@cknitt cknitt force-pushed the fix-bitwise-not-crash branch from 4a3fd8f to fa63fde Compare October 14, 2025 06:04
Copy link

pkg-pr-new bot commented Oct 14, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7965

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7965

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7965

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7965

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7965

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7965

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7965

commit: b012f97

@cometkim
Copy link
Member

It shouldn't be parsed as a bitwise operator there.

Also, the resolution may be conflicted with #7894

@cknitt
Copy link
Member Author

cknitt commented Oct 14, 2025

It shouldn't be parsed as a bitwise operator there.

Ah, right, bitwise should be ~~ (or ~~~ after #7894). And the original bug report was using ~~ anyway.

So we have two problems:

  1. Crashes when using bitwise not with incompatible types
  2. Single ~ being parsed as bitwise not

The first is already resolved by this PR. If we call unify, we also need to handle the Ctype.Unify exception correctly.

Also, the resolution may be conflicted with #7894

What exactly may be conflicted how?

@cknitt cknitt force-pushed the fix-bitwise-not-crash branch from b6a8dee to b012f97 Compare October 17, 2025 07:02
@cknitt cknitt requested a review from zth October 17, 2025 07:31
Comment on lines +3456 to +3463
| _ -> (
try
unify env lhs_type (instance_def Predef.type_int);
instance_def Predef.type_int
with Ctype.Unify trace ->
raise
(Error (lhs.exp_loc, env, Expr_type_clash {trace; context = None}))
)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed now but wasn't before...?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it was always needed, but for a long time nobody noticed that it would crash if you passed a non-matching type, until this was reported in #7960.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right. Good catch then!

@zth zth self-requested a review October 17, 2025 09:38
@cknitt cknitt merged commit 973351a into rescript-lang:master Oct 17, 2025
25 checks passed
@cknitt cknitt deleted the fix-bitwise-not-crash branch October 17, 2025 10:21
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 this pull request may close these issues.

using ~ incorrectly crashes compiler (12 rc1)

3 participants