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

BigInt typechecking #7602

Closed
wants to merge 23 commits into from
Closed

BigInt typechecking #7602

wants to merge 23 commits into from

Conversation

goodmind
Copy link
Contributor

@goodmind goodmind commented Mar 31, 2019

Fixes #6639
Library definitions #7534 (BigInt builtin and typed arrays)

Probably signature verification should be rewritten too, but I don't know what it does.
There maybe missing cases in something like refinements, assignment operators (especially since they seem untyped)

Adds sketchy-null-bigint lint rule and supports bigint type in sketchy-number with different lint message

@goodmind goodmind changed the title Bigint typechecking BigInt typechecking Mar 31, 2019
src/typing/statement.ml Outdated Show resolved Hide resolved
@goodmind
Copy link
Contributor Author

goodmind commented Apr 2, 2019

Not sure why tests fail, it is something about lsp tests

@mvitousek
Copy link
Contributor

Is there a reason that the names "bigint" and "bignum" are inconsistently used? I'd personally rather align everything with "bigint" since (as far as I understand it) that's the actual Flow type being provided.

@goodmind
Copy link
Contributor Author

goodmind commented Apr 10, 2019

@mvitousek just arbitrary, BigNum in typechecker, BigInt in AST, I just copypasted NumT with added Big 🙃

@goodmind
Copy link
Contributor Author

@mvitousek is this all that bothers you?

@goodmind
Copy link
Contributor Author

@mvitousek I renamed all of them to BigInt

@goodmind goodmind force-pushed the bigint-typecheck branch from accc410 to 33f0535 Compare June 7, 2019 07:54
@goodmind goodmind mentioned this pull request Jul 18, 2019
@samwgoldman samwgoldman mentioned this pull request Jul 26, 2019
64 tasks
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@mvitousek has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.


| BoolP (* boolean *)
| FunP (* function *)
| NumP (* number *)
| BIgIntP (* bigint *)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| BIgIntP (* bigint *)
| BigIntP (* bigint *)

@@ -22,13 +22,15 @@ let string_of_pred_ctor = function
| BoolP -> "BoolP"
| StrP -> "StrP"
| NumP -> "NumP"
| BIgIntP -> "BIgIntP"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| BIgIntP -> "BIgIntP"
| BigIntP -> "BigIntP"

@@ -1388,6 +1400,7 @@ and json_of_pred_impl json_cx p = Hh_json.(
| StrP
| SymbolP
| NumP
| BIgIntP
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| BIgIntP
| BigIntP

(* typeof _ ~ "bigint" *)
(***********************)

| BIgIntP ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| BIgIntP ->
| BigIntP ->

| BIgIntP ->
rec_flow_t cx trace (Type_filter.bigint l, t)

| NotP BIgIntP ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| NotP BIgIntP ->
| NotP BigIntP ->

@@ -5537,6 +5649,7 @@ and predicates_of_condition cx e = Ast.(Expression.(
| "boolean" -> Some BoolP
| "function" -> Some FunP
| "number" -> Some NumP
| "bigint" -> Some BIgIntP
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| "bigint" -> Some BIgIntP
| "bigint" -> Some BigIntP

@@ -201,156 +201,7 @@ export default suite(({ addFile, addFiles, addCode }) => [
const valid_neg_small = -100n;
type ValidSmall = 100n;
type ValidNegSmall = -1n;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests for BigInts with numeric separators are missing here. Parser already support them.

@gkz
Copy link
Member

gkz commented Dec 14, 2022

BigInt is now supported as of 0.194

@gkz gkz closed this Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add support for new type: bigint
6 participants