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

basic typechecking #568

Merged
merged 22 commits into from
Jan 17, 2024
Merged

basic typechecking #568

merged 22 commits into from
Jan 17, 2024

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented Jan 7, 2024

Catches errors like tc39/proposal-joint-iteration#11 (comment) via a very basic type system.

The checks only apply to call arguments, and specifically only to arguments which are either literals or themselves calls (including ! Foo()-style unwrapped calls). There's no tracking of variables or anything yet.

Also, we can't generally infer types precisely. For example, imagine Identity(x) which returns its input and accepts any value: TakesInteger(Identity(42)) would be a reasonable thing to write, but the type system here is much to weak to know that. So we check only that the intersection (that is, the meet) of the argument type and parameter type is nonempty.

The first commit is just moving code around. I recommend reviewing commits individually.

@bakkot bakkot requested a review from michaelficarra January 7, 2024 03:08
@bakkot bakkot force-pushed the check-call-args branch 2 times, most recently from e02b455 to d84d01a Compare January 7, 2024 07:17
(a.kind === 'integer' && b.kind === 'concrete real') ||
(a.kind === 'integral number' && b.kind === 'concrete number')
) {
return !b.value.includes('.');
Copy link
Member

Choose a reason for hiding this comment

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

1.23456789e+23 is integral and a canonical ToString.

Suggested change
return !b.value.includes('.');
let n = parseFloat(b.value);
return n === Math.round(n);

Alternatively, you could just keep a number instead of a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we actually want to allow people to write numbers like *1.23456789e+23*<sub>𝔽</sub>?

Copy link
Member

Choose a reason for hiding this comment

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

Does IEEE-754 prescribe a way to refer to float values in documentation and such?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I can find on skimming, and it's too long for me to easily feed into an LLM to ask one to read it more thoroughly without chunking it up, which I'm too lazy to do.

Copy link
Member

Choose a reason for hiding this comment

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

I guess I would slightly lean toward only permitting DecimalDigits and DecimalDigits . DecimalDigits and requiring that they be an exactly-representable float value. This is an editorial question we should confirm at an editor call.

Regardless though, I would still change the representation to number and move the complexity of what's permitted upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretend I said 0.1 like I obviously meant.

Copy link
Member

Choose a reason for hiding this comment

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

That could be fine if we add some explanation in the Notational Conventions section saying that we mean the nearest-representable float. This is a topic for editor call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, changing this to be a number doesn't work for reals, which can in principle be values which are not representable as doubles. So I would still need to keep the string-handling logic for the real case. Given that, I don't want to introduce a different path just four doubles. That's a lot of extra complexity for no benefit.

Copy link
Member

Choose a reason for hiding this comment

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

You already have different paths for reals and doubles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In some places, but not in this case.

src/typechecker.ts Outdated Show resolved Hide resolved
@michaelficarra
Copy link
Member

You should add support for undefined/booleans.

test/typecheck.js Outdated Show resolved Hide resolved
@bakkot
Copy link
Contributor Author

bakkot commented Jan 9, 2024

You should add support for undefined/booleans.

Done.

@michaelficarra
Copy link
Member

It looks like you're missing some coverage in the tests. For example, I don't see anything for EcmaScript language values. I would expect the whole dominator lattice to be tested.

test/typecheck.js Outdated Show resolved Hide resolved
@bakkot bakkot merged commit 2c4fdd0 into main Jan 17, 2024
2 checks passed
@bakkot bakkot deleted the check-call-args branch January 17, 2024 03:20
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.

2 participants