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

[Test] Dump types for 'stdlib' and 'examples' #5450

Merged

Conversation

effectfully
Copy link
Contributor

Currently we only test that the type of each builtin from the default set of builtins is well-kinded and stays the same for the latest version. We should explicitly check all versions instead.

We also do not dump the types of definitions from stdlib and examples, we only check that those definitions are well-typed. We should dump the types, so that we can be sure those won’t suddenly change without us noticing.

Resolves PLT-6979.

@effectfully effectfully added the No Changelog Required Add this to skip the Changelog Check label Aug 2, 2023
Copy link
Contributor

@mjaskelioff mjaskelioff left a comment

Choose a reason for hiding this comment

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

As far as I can tell LGTM. Don't like the name unsafeCoerce for something that doesn't behave like unsafeCoerce. Especially because if you don't look at the details, it looks like PLC is not type-safe.

Copy link
Contributor

Choose a reason for hiding this comment

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

This file is called unsafeCoerce, because it has the type of Haskell's unsafeCoerce. What about its semantics?
Does it behave like unsafeCoerce as well? Or is it equivalent to

diverge : all b. b
diverge = ...
unsafeCoerce : all a b. a -> b
unsafeCoerce a = diverge

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw the comment on the other PR. It is indeed fix id, so while the typing corresponds to unsafeCoerce, it doesn't behave like it, so it's misleading to call it like that. It's not unsafe to execute it. Perhaps divergeCoerce would be more appropriate?

Copy link
Contributor Author

@effectfully effectfully Aug 3, 2023

Choose a reason for hiding this comment

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

Great point, I'll rename it. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@effectfully effectfully enabled auto-merge (squash) August 3, 2023 18:07
@michaelpj michaelpj disabled auto-merge August 9, 2023 09:16
@michaelpj michaelpj merged commit 5bd9d24 into master Aug 9, 2023
3 of 5 checks passed
@michaelpj michaelpj deleted the effectfully/test/dump-types-for-stdlib-and-examples branch August 9, 2023 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Required Add this to skip the Changelog Check Test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants