Skip to content

Fix for issue https://issues.scala-lang.org/browse/SI-9010 #58

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

Closed
wants to merge 1 commit into from
Closed

Fix for issue https://issues.scala-lang.org/browse/SI-9010 #58

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 6, 2015

No description provided.

@lrytz
Copy link
Member

lrytz commented Jul 7, 2015

@gourlaysama can you review this PR?

@gourlaysama gourlaysama self-assigned this Jul 7, 2015
This solution takes some ideas / inspiration from the discussion in
https://issues.scala-lang.org/browse/SI-4929.

Added LastNoSuccessHelper object into Parsers trait to
encapsulate tracking of the last NoSuccess result when
tracking is actually needed. Idea is that the last NoSuccess
result is only needed inside phrase parsers
to give better error messages i.e. it is not needed for "correctness".

Null is used as initial value for context:DynamicValue variable in
LastNoSuccessHelper object as a marker if computation is outside
"trackLastNoSuccessWithInitialValue" and therefore tracking
of NoSuccess results is unnecessary.

Tracking of NoSuccess results is done only iff
computation goes trough "trackLastNoSuccessWithInitialValue",
because then context variable inside LastNoSuccess helper
gets None(non-null) value which enables tracking.
@ghost ghost closed this Jul 11, 2015
@ghost ghost changed the title Fix for issue https://issues.scala-lang.org/browse/SI-9010. . Jul 12, 2015
@ghost ghost changed the title . .Fix for issue https://issues.scala-lang.org/browse/SI-9010 Jul 12, 2015
@ghost ghost changed the title .Fix for issue https://issues.scala-lang.org/browse/SI-9010 Fix for issue https://issues.scala-lang.org/browse/SI-9010 Jul 12, 2015
@ghost
Copy link
Author

ghost commented Jul 12, 2015

I close this pull request and make a better one.
See : #59

This pull request was closed.
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.

3 participants