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

support explicit subtyping #236

Merged
merged 1 commit into from
Mar 31, 2019
Merged

support explicit subtyping #236

merged 1 commit into from
Mar 31, 2019

Conversation

davidchambers
Copy link
Member

@davidchambers davidchambers commented Mar 26, 2019

Before:

var FiniteNumber = $.NullaryType
  ('FiniteNumber')
  ('https://github.com/sanctuary-js/sanctuary-def/tree/v0.19.0#FiniteNumber')
  (function(x) { return ValidNumber._test (x) && isFinite (x); });

After:

var FiniteNumber = $.NullaryType
  ('FiniteNumber')
  ('https://github.com/sanctuary-js/sanctuary-def/tree/v0.19.0#FiniteNumber')
  ([ValidNumber])
  (isFinite);

The primary reason for this change is to make defining subtypes more convenient. In the future the relationships this change exposes could be used to make type checking more efficient. If a value is not a member of $.Number, say, we know it is not a member of any of the subtypes of $.Number either.

@davidchambers davidchambers requested a review from Avaq March 26, 2019 17:48
@davidchambers
Copy link
Member Author

⚠️ Before this pull request is merged, we must decide what to do with record types.

Currently, record types are shown in { name :: type, name :: type } form in type signatures and error messages. In some cases this makes it possible to indicate which field is incorrectly typed.

Record types can now subtype other record types. We could make code changes to ensure that $.RecordType ([$.RecordType ([]) ({x: X})]) ({y: Y}) behaves identically to $.RecordType ([]) ({x: X, y: Y}). This special case would not apply to record types defined as subtypes of non-record types. This is an edge case, admittedly, but imagine reading an error message stating that {y: 0} is not a member of { y :: Number } (due to an unacknowledged dependency). Confusing.

One option is to leave $.RecordType as it exists on master, only capable of defining anonymous record types without supertypes. We would add a second type constructor, $.NamedRecordType, that would also take name, url, and supertypes. Types defined via this constructor would be referred to by name in type signatures and error messages, avoiding the problem @JAForbes noted in #138.

@Avaq
Copy link
Member

Avaq commented Mar 27, 2019

This seems like the first in a series of steps towards the overly ambitious goals I set for myself with #162. Maybe it's time to close that PR, seeing as the branch has fallen quite far behind, and incremental steps probably will get us there faster.

@davidchambers
Copy link
Member Author

Aha! I thought we had discussed this. I searched issues but not pull requests, so failed to find #162.

@davidchambers
Copy link
Member Author

Let's merge #238 before taking further action here.

@davidchambers
Copy link
Member Author

This pull request is now ready to be reviewed. :)

@davidchambers davidchambers force-pushed the davidchambers/subtyping branch 2 times, most recently from 7cc678f to ec58641 Compare March 29, 2019 18:32
@davidchambers davidchambers merged commit 546fc10 into master Mar 31, 2019
@davidchambers davidchambers deleted the davidchambers/subtyping branch March 31, 2019 16:12
@@ -1677,6 +1761,8 @@
//.
//. To define an anonymous record type one must provide:
//.
//. - an array of supertypes (exposed as `t.supertypes`); and
//.
Copy link
Member Author

Choose a reason for hiding this comment

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

This was an erroneous inclusion. I will remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

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