Implements the Never ! types as a TypeInfo bottom type.
#5602
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
esdrubal
added
language feature
esdrubal
force-pushed
the
esdrubal/typeinfo_never
branch
3 times, most recently
from
1e3a257
to
f8ca1c7
Compare
We now parse the `!` as a TypeInfo::Never, and remove the usage of empty enums as Never type in our code. This commit removes completely the DeterministicallyAborts and TypeCheckUnificationContext. The DeterministicallyAborts can be removed because the Never TypeInfo is now propagated using the type checker. Code blocks that return, break, continue, or call an expression that returns Never, are marked as Never. Partially fixes #5562.
IGI-111
requested changes
Feb 12, 2024
There was a problem hiding this comment.
I do so love PRs with negative line totals. Getting rid of all this complex logic in favor of a defined type is nice. The end result looks a lot cleaner.
But I'm about to ruin it by asking you to write some documentation for it.
! is a fairly complex concept and the book should absolutely cover it.
ironcev
requested changes
Feb 12, 2024
There was a problem hiding this comment.
Awesome!! Can you please check the questions regarding the coercion and equality on the IR level?
Also, assuming how crucial Never is for the type system, I would expect new dedicated tests. E.g., testing coercion rules could be an example, having ! as a type in different positions, e.g., as a function parameter, etc.
|
BDW solving #5560 would be a great test of the new type system (as a separate PR of course). I wouldn't be surprised if the bug is no more reproducible on this branch 😄 |
@IGI-111 Completely agree about the need for the docs! But want to selfishly ask if the documentation could be a part of a separate PR if that will speed up the merge of this PR. I was doing changes in the |
|
Okay sure, if that blocks some other work we can split it into another PR. |
jjcnn
requested changes
Feb 13, 2024
Added docs for Never type to the Sway book.
Added more coercion positive and negative tests. About the |
Benchmark for bb163e2Click to view benchmark
|
Benchmark for 057388cClick to view benchmark
|
ironcev
previously requested changes
Feb 16, 2024
esdrubal
force-pushed
the
esdrubal/typeinfo_never
branch
2 times, most recently
from
e429daa
to
bd61bf7
Compare
Benchmark for d7eaee9Click to view benchmark
|
jjcnn
approved these changes
Feb 19, 2024
As it happens this issue blocks the one I'm currently working on, so it would be nice to have it merged asap. |
7 tasks
IGI-111
approved these changes
Feb 19, 2024
Benchmark for ae537d3Click to view benchmark
|
IGI-111
pushed a commit
that referenced
this pull request
Feb 27, 2024
…#5631) ## Description Fixes #5616 and #5622. When generating the IR for constant and variable declarations the name being declared is not added to the local environment if the initializer cannot be compiled. This can happen when initializing a constant using a non-constant expression. Since the name isn't added to the environment, and since we attempt to continue compilation after an error, any attempt to use the name later in the code will cause the name to not be found, which in turn causes the IR converter to throw an ICE because it assumes that all used names are in scope. This PR fixes the issue by not throwing the initializer error until the name has been added to the environment. Note that 1. The initializer must be compiled before the name is added to the environment. The name should not be in scope during its own initialization. 2. #5602 is blocking a more elegant solution to this problem. The lack of a `Never` type means that the call to `convert_resolved_typeid` may fail if the initializer diverges, and since that call must be made before the new name can be added to the environment we check for termination first. The order of operations is therefore currently a) compile the initializer, then b) check that the initializer did not result in an error and did not diverge, then c) resolve the type of the name being declared, then d) add the name to the environment, and finally e) throw an error if the initializer resulted in an error. Once the `Never` type is introduced we can move the terminator check to the end of that sequence. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We now parse the
!as a TypeInfo::Never, and remove the usage of empty enums as Never type in our code.This commit removes completely the DeterministicallyAborts and TypeCheckUnificationContext.
The DeterministicallyAborts can be removed because the Never TypeInfo is now propagated using the type checker. Code blocks that return, break, continue, or call an expression that returns Never, are marked as Never.
Partially fixes #5562.
Checklist
Breaking*orNew Featurelabels where relevant.