-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
feat(compiler): Providing, including, reproviding exceptions #1849
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just had one small nit.
@@ -11,7 +11,7 @@ include /* special include */ "array" | |||
include "array" as /* special include */ Foo | |||
from List use { length, map, forEach as each } | |||
from Opt use { MutableOpt, ImmutableOpt as Imm, type Opt, type Opt as OptAlias } | |||
from Opt use { MutableOpt, /* comment1 */ ImmutableOpt /* comment2 */ as /* comment3 */ Imm /* comment4 */, /* comment5 */ type /* comment6 */ Opt, type Opt as /* comment7 */ OptAlias } | |||
from Opt use { MutableOpt, /* comment1 */ ImmutableOpt /* comment2 */ as /* comment3 */ Imm /* comment4 */, /* comment5 */ type /* comment6 */ Opt, type Opt as /* comment7 */ OptAlias, exception Exc as /* comment8 */ E } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For completeness and to follow the rest it would probably make sense to add the test here without as
as well to mirror the variable and type import.
from Opt use { MutableOpt, /* comment1 */ ImmutableOpt /* comment2 */ as /* comment3 */ Imm /* comment4 */, /* comment5 */ type /* comment6 */ Opt, type Opt as /* comment7 */ OptAlias, exception Exc as /* comment8 */ E } | |
from Opt use { MutableOpt, /* comment1 */ ImmutableOpt /* comment2 */ as /* comment3 */ Imm /* comment4 */, /* comment5 */ type /* comment6 */ Opt, type Opt as /* comment7 */ OptAlias, , exception /* comment8 */ Exc, exception Exc as /* comment9 */ E } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some tests where you provide an exception + a value of that exception and then successfully pattern match on it in the consuming module? Just want to make sure that works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo @ospencer's test request
as
when you use the from syntax with
I feel like this might be a little confusing the closest analogy I can think of would be if we allowed enum aliasing, you could do something like I wonder if we want to allow aliasing exceptions. Cut down version of message sent in discord. Also we should add a test for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks useful to me and good syntax.
62bb03c
to
5ba856f
Compare
5ba856f
to
2b7be89
Compare
2b7be89
to
bfa4728
Compare
Closes #1053
Syntax: