Fix #354 by relaxing %expect to check for upper bound

[sgraf812]

No description provided.

[phadej]

This is arguably a major semantic change of %expect. Some people for sure rely on %expect being strict, so unnoticed removed conflicts wouldn't allow addition of completely new conflicts. I very vaguely remember that GHC used to do exactly that.

And warning is next to useless. I have never seen cabal-install print any preprocessor (happy or alex) warning. Hopefully it is because grammars were warning free, not because cabal-install simply omits the preprocessor diagnostic outputs

EDIT: yes,

commit 87e2e2b17afed82d30841d5b44c977123b93ecc4
Author: Vladislav Zavialov <vlad.z.4096@gmail.com>
Date:   Tue Aug 25 18:18:12 2020 +0300

    Resolve shift/reduce conflicts with %shift (#17232)

The change in GHC is relatively recent.

I'd argue that happy should had first warned people to not use non-zero %expect, but rather use %shifts.

---

I'm dislike this PR been done quick and in minor version; it breaks my trust in happy as a tool (which is already quite weak TBH, I see happy as a tool to implement GHC, not really a general parser generator).

[andreasabel]

so unnoticed removed conflicts wouldn't allow addition of completely new conflicts.

That can also happen when %expect is strict, only with lower probability.

[andreasabel]

CI failure here: https://github.com/haskell/happy/actions/runs/24334489926/job/71047831338?pr=355#step:23:35

make: *** No rule to make target 'check.expect_upper.y', needed by 'all'.

[andreasabel]

I think this is a reasonable relaxation of the %expect semantics.

[phadej]

That can also happen when %expect is strict, only with lower probability.

So?

If people want strict check, let them have one. Personally I'd be fine if %expect_atmost pragma is added and %expect is left unchanged (and strict).

Let's nor argue about what's best option if we can have both. And let's not change existing features which people may heavily rely upon in minor versions.

It's unfortunate that a bug fix (#353) changes things in a breaking way, but that's a reality of software development. Sometimes even a somewhat small bug fix is actually a breaking change which warrants an major version. By making more major changes in minor versions you would just make things worse (= break trust that minor updates of happy versions are "safe").

[andreasabel]

And let's not change existing features which people may heavily rely upon in minor versions.

Ah yes, totally. Apologies, I missed that point. I agree this cannot be 2.2.2, but must be 2.3.

[sgraf812]

I will add %expect_strict and bump to 2.3.

But note that neither %expect nor %expect_strict are good solutions for the problem they address (acknowledging s/r conflicts and their default resolution). The way to do it properly is using %shift annotations on the conflicting production rule. GHC's parser has been using it for a long time and never looked back. Probably worth a sentence in the manual; will do.

[phadej]

But note that neither %expect nor %expect_strict are good solutions for the problem they address (acknowledging s/r conflicts and their default resolution).

As I said in my previous comment

I'd argue that happy should had first warned people to not use non-zero %expect, but rather use %shifts.