-
Notifications
You must be signed in to change notification settings - Fork 379
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
Add type annotations to monadic bind #3327 #3329
Merged
Merged
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
foo : IO Nat | ||
|
||
test1 : IO () | ||
test1 = do | ||
x : Nat <- foo | ||
printLn x | ||
|
||
test2 : IO () | ||
test2 = do | ||
1 x : Nat <- foo | ||
printLn x | ||
|
||
test3 : IO () | ||
test3 = do | ||
0 x : Nat <- foo | ||
printLn x | ||
|
||
test4 : IO () | ||
test4 = do | ||
1 x <- foo | ||
printLn x | ||
|
||
test5 : IO () | ||
test5 = do | ||
0 x <- foo | ||
printLn x | ||
|
||
bar : IO (Maybe Nat) | ||
|
||
test6 : IO () | ||
test6 = do | ||
Just x : Maybe Nat <- bar | ||
| Nothing => pure () | ||
printLn x | ||
|
||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
1/1: Building Issue3327 (Issue3327.idr) | ||
Error: While processing right hand side of test2. Trying to use linear name x in non-linear context. | ||
|
||
Issue3327:11:11--11:12 | ||
07 | | ||
08 | test2 : IO () | ||
09 | test2 = do | ||
10 | 1 x : Nat <- foo | ||
11 | printLn x | ||
^ | ||
|
||
Error: While processing right hand side of test3. x is not accessible in this context. | ||
|
||
Issue3327:16:11--16:12 | ||
12 | | ||
13 | test3 : IO () | ||
14 | test3 = do | ||
15 | 0 x : Nat <- foo | ||
16 | printLn x | ||
^ | ||
|
||
Error: While processing right hand side of test4. Trying to use linear name x in non-linear context. | ||
|
||
Issue3327:21:11--21:12 | ||
17 | | ||
18 | test4 : IO () | ||
19 | test4 = do | ||
20 | 1 x <- foo | ||
21 | printLn x | ||
^ | ||
|
||
Error: While processing right hand side of test5. x is not accessible in this context. | ||
|
||
Issue3327:26:11--26:12 | ||
22 | | ||
23 | test5 : IO () | ||
24 | test5 = do | ||
25 | 0 x <- foo | ||
26 | printLn x | ||
^ | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
. ../../../testutils.sh | ||
|
||
check Issue3327.idr |
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.
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.
Isn't it bizarre that we are using
desugarDo
to elaborate the type?Does it even make sense to have an effectful function returning a type here?
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.
Yeah, that struck me as odd, but I followed the pattern of
DoLet
andDoLetPat
. It looks like they were changed from a fulldesugar
todesugarDo
in ee063a5 to propagate thedo
namespace.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.
Right, I think that should be reverted.
Unless @Russoul has a motivation for it.
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.
I'll let you guys sort out what is wanted here. It looks like
desugar
simply callsdesugarDo
with a namespace ofNothing
.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.
Yeah I think the function name threw me off, but either looks fine. The difference seems to be what namespace ends up on comprehensions, which I wouldn't expect to appear in types.
What did you want to revert? Is it just changing the other
ty
elaborations todesugar
or more of #1700?