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.

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

version 2.1: unrecognised directive: %errorhandlertype #320

Closed
yav opened this issue Oct 18, 2024 · 11 comments · Fixed by #322
Closed

version 2.1: unrecognised directive: %errorhandlertype #320

yav opened this issue Oct 18, 2024 · 11 comments · Fixed by #322

Comments

@yav
Copy link
Contributor

yav commented Oct 18, 2024

It appears that as of version 2.1 happy reports:

unrecognised directive: %errorhandlertype

This directive is still in the docs, so if it is removed indeed, it'd be nice to update the docs to say what changed, and what to do instead.

@int-index
Copy link
Collaborator

Its removal is not in the changelog, so I'd assume it was unintentional.

#318 is likely the culprit.

@Ericson2314
Copy link
Collaborator

https://github.com/haskell/happy/pull/318/files#diff-afd5c13988aad40ea499045febecb363cc647906c5241ae478543521d39b5daa OK that test update looks it is the culprit

CC @sgraf812 you mentioned changing an undocumented thing, but this is different?

@sgraf812
Copy link
Collaborator

Thanks for the bug report and sorry for inflicting it upon you on a Friday! I deprecated 2.1.

I was not aware that the old %errorhandlertype was documented (it appears I looked in the wrong file ...), so I removed it without replacement. It is superseded by the simple flag %error.expected.

I had thought I could simply bring back the old flag in a backwards compatible way, but the feature was implemented weirdly enough that this is not as simple to achieve.

I'm inclined to bump the version of happy to 3.0 and emit a warning that %errorhandlertype was overhauled and superseded by %error.expected.

@sgraf812
Copy link
Collaborator

@yav I would be glad if you gave the user guide changes in #322 a read to see if they make sense to you. It appears you are a very rare client of this feature :) I assure you that its functionality in 3.0 has been improved.

@yav
Copy link
Contributor Author

yav commented Oct 19, 2024

The description of %error.expected is nice, as a standalone feature. I don't know the technical details, but from the description it sounds like %error.expected does a very similar thing to %errorhandlertype expect%.

I imagine that %error.expected turns on some boolean flag that is used by happy when it detects errors. Is there really no way to set the same internal flag with %errorhandlertype expect%, instead of having to rename the directive?

I think this would be much better, because we'd remain somewhat backward compatible, even if the new behavior is a bit different somehow. And, if it is, then the docs should emphasize the difference to the old behavior, at least for a little while, until the old behavior is far enough in the past that is unlikely ti be used.

@sgraf812
Copy link
Collaborator

I don't know the technical details, but from the description it sounds like %error.expected does a very similar thing to %errorhandlertype expect%.

Indeed, I intended to fix a few things in the implementation of %errorhandlertype:

  • (minor) Interface-wise, it should really be a boolean flag, one that composes well with other flags of its kind (such as the two-action form of the %error directive for resumptive parsing introduced in 2.1). It does not make sense to introduce an enumeration to pass as a "parameter" to %errorhandlertype.
  • (major) The technique used to generate the list of expected tokens was both technically incomplete and quite space-inefficient (happyExpToks table can be larger than happyTable #271).

Neither issue would require a backwards incompatible change.

However, the main problem (and the reason for the incompatibility) is that %errorhandlertype explist passes the additional token list parameter to the error handler by turning the Token parameter into (Token, [String]) -> P a. The main payload of 2.1 is the new catch mechanism for resumptive parsing, which requires passing the resumption as a parameter to the error handler. I don't think it's good design to turn the pair into a triple (Token, [String], P a) -> P a, where the first P a is the resumption, and then into a quadruple when the next extension to happy comes along. I would rather pass these parameters individually, Token -> [String] -> P a -> P a.

However I can see that backwards compatibility is an important virtue. Perhaps I can try to mimic the old behavior when %errorhandlertype explist is used. Presumably with resumptions we would get (Token, [String]) -> P a -> P a, but that would work with Happy 2.1 anyway in which case one could use %error.expected instead. Your (catch-free) use case with %errorhandlertype explist would keep working across multiple versions of Happy.

Will try something on Monday.

@sgraf812
Copy link
Collaborator

I managed to bring back %errorhandlertype in #322. It's ugly but does the job (back-compat). Hopefully nobody uses this feature anymore in 2 years time.

@jmcardon
Copy link

jmcardon commented Oct 21, 2024

Just as an extra sample point, this broke our builds as well here. We also used %errorhandlertype.

We've tried to fix it but currently there seems to be instability based on published builds, and now %error.expected causes a lexical error:

pact/Pact/Core/Syntax/Parser.y:44: lexical error before `.'
Error: cabal: repl failed for pact-tng-5.0.

@sgraf812
Copy link
Collaborator

sgraf812 commented Oct 22, 2024

@jmcardon Thanks for the feedback. It's good to know that people seem to actually use the %errorhandlertype feature!

Can you try again with the following diff to your cabal.project? (Perhaps you also need to relax the upper bound on happy)

diff --git a/cabal.project b/cabal.project
index 565e3d93..65c7df21 100644
--- a/cabal.project
+++ b/cabal.project
@@ -1,4 +1,7 @@
-packages: .
+packages:
+  .
+  http://hackage.haskell.org/package/happy-lib-2.1.1/candidate/happy-lib-2.1.1.tar.gz
+  http://hackage.haskell.org/package/happy-2.1.1/candidate/happy-2.1.1.tar.gz
 
 package pact-tng
     -- When running Template Haskell, GHC sees that some code it loads has dependent modules

I'll hold back with the 2.1.1 release until tomorrow or until you can confirm to me that it works.

@jmcardon
Copy link

@sgraf812 I added the upper bound due to the build errors.

After relaxing the upper bound and changing our cabal.project, indeed it works! Thank you!

@sgraf812
Copy link
Collaborator

Great. I just released 2.1.1.

Be sure to check out the new catch feature documented in the user's guide!

RyanGlScott added a commit to GaloisInc/language-rust that referenced this issue Jan 6, 2025
`happy-2.1.1` includes a fix for haskell/happy#320,
which was preventing `language-rust` from building. Now that this version of
`happy` is on Hackage, we no longer need to include such a restrictive upper
version bound on `happy`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants