Skip to content

Add support for the bitwise NOT(~) unary operator #7418

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 5, 2025
Merged

Add support for the bitwise NOT(~) unary operator #7418

merged 5 commits into from
May 5, 2025
+178 −78

Conversation

cometkim
Copy link
Member

@cometkim cometkim commented May 4, 2025

Part of the #7172

Allows to use ~ in unary expressions, and make it as compiler primitive instead of using runtime library code

There is one syntax ambiguity problem with function arguments as we already use tilde token for the labeled argument syntax, e.g.

call(~a)
//   ^ is it for passing labeled argument `a`?
//   ^ or is it for passing bitwise NOT of `a` value

So we decided to enforce parens on that case.

call(~a)
// is for passing labeled arguments

call((~a))
// is for passing bitwise NOT of `a`, formatter will not force unparen for this specific expression.

And also we add parens in binary expressions too:

~a ^ b
// be formatted into
(~a) ^ b

Note: unlike existing + and -, ~n is not being parsed as constant, only expression

@cometkim cometkim requested a review from cknitt May 4, 2025 13:43
@cometkim cometkim mentioned this pull request May 3, 2025
Open
4 tasks
@cometkim cometkim changed the title Add support for the bitwise NOT() unary operator Add support for the bitwise NOT(~) unary operator May 4, 2025
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented May 4, 2025 •
edited
Loading

Open in StackBlitz

rescript

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/win32-x64

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

@rescript/darwin-arm64

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

commit: 7983648

cknitt
cknitt reviewed May 4, 2025
View reviewed changes
Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

Nice work! 🎉 Just left some comments.

@cristianoc could you review, too?

compiler/syntax/src/res_core.ml
@@ -268,7 +268,7 @@ let rec go_to_closing closing_token state =
(* Madness *)
let is_es6_arrow_expression ~in_ternary p =
Parser.lookahead p (fun state ->
let async =
let _async =
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why we didn't get a warning before if this is unused?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was used for recovery on tilde token, I forgot to rollback the recovery change

Copy link
Member Author

@cometkim cometkim May 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe not. we should remove the tilde handling because now we allow to use it somewhere in that context.

@cknitt cknitt requested a review from cristianoc May 4, 2025 15:20
cristianoc
cristianoc reviewed May 4, 2025
View reviewed changes
tests/syntax_tests/data/parsing/grammar/expressions/expected/apply.res.txt
@@ -6,4 +6,7 @@
;;List.reduce (fun [arity:2]acc -> fun curr -> acc + curr) 0 myList
let unitUncurried = apply ()
;;call ~a:(a : int)
;;call (~ a)
;;call ~a:(a ^ (~ a))
;;call ~a:(~ a)
;;call_partial 3 ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add some simple examples too? E.g. let z = ~y. To illustrate where parens are added, and where they are not.

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 has been added in the unary.res file already?

cristianoc
cristianoc approved these changes May 4, 2025
View reviewed changes
Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

Looks good to me. Left a minor question.

cometkim
cometkim commented May 4, 2025
View reviewed changes
compiler/syntax/src/res_core.ml
@@ -315,7 +315,6 @@ let is_es6_arrow_expression ~in_ternary p =
| EqualGreater -> true
| _ -> false)
| Dot (* uncurried *) -> true
| Tilde when not async -> true
Copy link
Member Author

@cometkim cometkim May 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Here where we use the async bool value before

@cometkim
Copy link
Member Author

cometkim commented May 4, 2025

I'm merging this and continuing to work on the |.

Let me know if there's still something that needs to be fixed

@cometkim cometkim merged commit c65c6c0 into rescript-lang:master May 5, 2025
21 checks passed
@cometkim cometkim deleted the op-bnot branch May 5, 2025 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cknitt cknitt cknitt left review comments

@cristianoc cristianoc cristianoc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cometkim @cknitt @cristianoc