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

Spec: Refine the opt rule #246

Merged
merged 3 commits into from
Aug 30, 2021
Merged

Spec: Refine the opt rule #246

merged 3 commits into from
Aug 30, 2021

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Jun 16, 2021

this writes the rules out as described in
#168 (comment)

I believe that this is necessary for completeness, as shown by this
table, which checks that t <: opt t' is covered:

| null <: t | null <: t' | t <: t' | Rule?
| --- | --- | --- | --- |
| no | no | no | second rule
| no | no | yes | first rule
| no | yes | no | third rule
| no | yes | yes | third rule
| yes | no | no | second rule
| yes | no | yes | impossible (<: is transitive)
| yes | yes | no | see rules for  `t = null`, `t = opt …` or `t = reserved`
| yes | yes | yes | see rules for  `t = null`, `t = opt …` or `t = reserved`

Remember that null <: t implies that t = null, t = opt … or t = reserved, and these cases have their own rules.

Fixes #239.

this writes the rules out as described in
#168 (comment)

I believe that this is necessary for completeness, as shown by this
table, which checks that `t <: opt t'` is covered:

```
| null <: t | null <: t' | t <: t' | Rule?
| --- | --- | --- | --- |
| no | no | no | second rule
| no | no | yes | first rule
| no | yes | no | third rule
| no | yes | yes | third rule
| yes | no | no | second rule
| yes | no | yes | impossible (<: is transitive)
| yes | yes | no | see rules for  `t = null`, `t = opt …` or `t = reserved`
| yes | yes | yes | see rules for  `t = null`, `t = opt …` or `t = reserved`
```

Remember that `null <: t` implies that `t = null`, `t = opt …` or `t =
reserved`, and these cases have their own rules.

Fixes #239.
@nomeata nomeata changed the title SpeC: Refine the opt rule Spec: Refine the opt rule Aug 27, 2021
@nomeata
Copy link
Collaborator Author

nomeata commented Aug 27, 2021

Shall we get that in?

@chenyan-dfinity
Copy link
Contributor

Fixed a typo for null <: t'. PTAL.

@nomeata
Copy link
Collaborator Author

nomeata commented Aug 30, 2021

Is it really a typo? Oh man, it’s always a knot in the brain to solve these, especially with the negations… but I think you are right, this way this matches the line

 C[<t> <: opt <t'>](_) = null  if not (null <: <t'>) and not (<t> <: <t'>)

in the rules for coercion.

I tried to peek into the Coq rules, but there we just have t <: opt t' without constraint, which is IMHO more honest, but I think Andreas didn’t want to simple write that in the Spec as it does not explain the fine points around subtyping well.

@chenyan-dfinity
Copy link
Contributor

Yeah, the Rust implementation has this condition, and if I change it to null <: t, the test suite fails.

@chenyan-dfinity chenyan-dfinity merged commit f3b0934 into master Aug 30, 2021
@chenyan-dfinity chenyan-dfinity deleted the joachim/fix-opt-rule branch August 30, 2021 16:44
ninegua pushed a commit to ninegua/candid that referenced this pull request Apr 22, 2022
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.

Update opt rule for subtyping and coercion
2 participants