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.

Sign up for GitHub

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

Jump to bottom

docs: add Queens effect example #7252

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

docs: add Queens effect example #7252

wants to merge 26 commits into from

Conversation

jaschdoc
Copy link
Member

Not sure if it's usable as an example. Still needs documentation if we decide to merge it in.

Related to #7093
Related to #7218

@jaschdoc jaschdoc changed the title docs: add Queens effect example docs: add Queens effect example Feb 19, 2024
@jaschdoc jaschdoc marked this pull request as ready for review February 19, 2024 14:47
JonathanStarup
JonathanStarup requested changes Feb 28, 2024
View reviewed changes
Copy link
Contributor

@JonathanStarup JonathanStarup left a comment

Choose a reason for hiding this comment

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

Seems cool to me, you can ping me again if you add documentation :)

JonathanStarup
JonathanStarup reviewed Apr 23, 2024
View reviewed changes
examples/effects/Queens.flix Outdated Show resolved Hide resolved
examples/effects/Queens.flix Show resolved Hide resolved
examples/effects/Queens.flix Outdated Show resolved Hide resolved
examples/effects/Queens.flix Outdated Show resolved Hide resolved
examples/effects/Queens.flix Outdated Show resolved Hide resolved
examples/effects/Queens.flix Outdated Show resolved Hide resolved
@jaschdoc
Copy link
Member Author

Also, @herluf-ba Parser2 fails on this example

@magnus-madsen
Copy link
Member

Also, @herluf-ba Parser2 fails on this example

Can you open separate ticket? Thanks!

@jaschdoc jaschdoc mentioned this pull request Apr 25, 2024
Closed
@herluf-ba
Copy link
Contributor

@jaschdoc I had a look. The reason is the doc-comments above fail and pick. If you turn them into regular line-comments then Parser2 seems to settle down.

Parser just allowed doc-comments (or any comment really) everywhere, while we are trying to impose just a few rules about comments with Parser2. One of them being, that doc-comments are only supposed to go above a declaration (a module, definition, enum and so on).

Another one is a couple of restrictions to block comments:

def main(): Int32 = 
  if /* This is no longer ok */ (true) 1 else 2

@jaschdoc
Copy link
Member Author

Parser just allowed doc-comments (or any comment really) everywhere, while we are trying to impose just a few rules about comments with Parser2. One of them being, that doc-comments are only supposed to go above a declaration (a module, definition, enum and so on).

Why are effect handler defs not considered declarations?

@jaschdoc
Copy link
Member Author

Also, from commit d678d79 I got this

ca.uwaterloo.flix.util.InternalCompilerException: [examples/effects/Queens.flix:107:1] Parser is stuck
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$nth(Parser2.scala:336)
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$atAny(Parser2.scala:360)
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$comments(Parser2.scala:787)
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$close(Parser2.scala:266)
	at ca.uwaterloo.flix.language.phase.Parser2$Decl$.docComment(Parser2.scala:1263)
	at ca.uwaterloo.flix.language.phase.Parser2$Decl$.declaration(Parser2.scala:882)
	at ca.uwaterloo.flix.language.phase.Parser2$.root(Parser2.scala:806)
	at ca.uwaterloo.flix.language.phase.Parser2$.parse(Parser2.scala:146)
	at ca.uwaterloo.flix.language.phase.Parser2$.$anonfun$run$2(Parser2.scala:133)
	at ca.uwaterloo.flix.util.ParOps$.$anonfun$parMap$2(ParOps.scala:64)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1423)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
(examples/effects/Queens.flix:107:1,CurlyR)
#
# An unexpected error has been detected by the Flix compiler:
#
#   [examples/effects/Queens.flix:107:1] Parser is stuck
#
# This is a bug in the Flix compiler. Please report it here:
#
# https://github.com/flix/flix/issues
#
# -- Flix Compiler --
#
# Flix Version : 0.46.0
#   incremental: All
#
# -- Java Virtual Machine --
#
# JVM Version  : 21.0.1 (2023-10-17)
# JVM Vendor   : GraalVM Community
# JAVA_HOME    : /home/_/.sdkman/candidates/java/21.0.1-graalce
# System       : Linux
#
# -- Stack Trace --
ca.uwaterloo.flix.util.InternalCompilerException: [examples/effects/Queens.flix:107:1] Parser is stuck
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$nth(Parser2.scala:336)
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$atAny(Parser2.scala:360)
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$comments(Parser2.scala:787)
	at ca.uwaterloo.flix.language.phase.Parser2$.ca$uwaterloo$flix$language$phase$Parser2$$close(Parser2.scala:266)
	at ca.uwaterloo.flix.language.phase.Parser2$Decl$.docComment(Parser2.scala:1263)
	at ca.uwaterloo.flix.language.phase.Parser2$Decl$.declaration(Parser2.scala:882)
	at ca.uwaterloo.flix.language.phase.Parser2$.root(Parser2.scala:806)
	at ca.uwaterloo.flix.language.phase.Parser2$.parse(Parser2.scala:146)
	at ca.uwaterloo.flix.language.phase.Parser2$.$anonfun$run$2(Parser2.scala:133)
	at ca.uwaterloo.flix.util.ParOps$.$anonfun$parMap$2(ParOps.scala:64)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1423)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)

@jaschdoc jaschdoc mentioned this pull request Apr 27, 2024
Closed
@herluf-ba
Copy link
Contributor

herluf-ba commented Apr 30, 2024 •
edited

Why are effect handler defs not considered declarations?

That's a good question. Conceptually I guess they are... I'll make note of it, maybe we need to allow doc comments on effect handlers in the future.

@jaschdoc
Copy link
Member Author

Personally I think that would make sense. Especially if the handler is nontrivial. But then maybe you would want to refactor that into a function that the handler calls... but I guess you can't expect users to do that so better to just give them the opportunity to write some documentation at the handler site. 😀

@magnus-madsen
Copy link
Member

Effect handlers are inside expressions. Not at the top level. No different from match.

@jaschdoc
Copy link
Member Author

Effect handlers are inside expressions. Not at the top level. No different from match.

True from a language design perspective but I still think users would see a def keyword and figure they could write a doc comment there.

@herluf-ba
Copy link
Contributor

@magnus-madsen I think this is ready to merge with #7643 closed.

@magnus-madsen
Copy link
Member

Thanks-- I am not merging it yet for other reasons: Still waiting for set-based effects to arrive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonathanStarup JonathanStarup JonathanStarup approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jaschdoc @magnus-madsen @herluf-ba @JonathanStarup