emilaxelsson / syntactic

Generic representation and manipulation of abstract syntax
BSD 3-Clause "New" or "Revised" License
25 stars 13 forks source link

Fix Project prj: catch-all was causing universal Nothing results. #35

Closed kquick closed 5 years ago

kquick commented 5 years ago

The addition of the catch-all for Project (https://github.com/emilaxelsson/syntactic/commit/9b1ddf2e) causes prj to always return Nothing because the catch-all is seen as a substitution instance for the :+: composition instances.

Added explicit unit tests to validate prj functionality.

kquick commented 5 years ago

A couple of additional notes:

emilaxelsson commented 5 years ago

Thanks for fixing this!

I'll fix the Travis build and try to make sure that recent GHCs are supported.

emilaxelsson commented 5 years ago

@kquick Did you forget to add the file SyntaxTests.hs?

kquick commented 5 years ago

@emilaxelsson looks like I did, sorry about that! It's been added now.

emilaxelsson commented 5 years ago

Good, thanks! I'm still working on getting a green build on Travis.

emilaxelsson commented 5 years ago

Just an update: I'm making slow progress. The failures turned out to be due to incorrectly specified overlap, combined with a GHC bug (see comment 3, which shows what happens in Syntactic).

Now the tests pass on GHC 8.4, but I get an infinite loop on 8.2 (and Travis shows a failing test on 8.2). Once I've resolved testing on GHC 8.2, I'll upload a new version.

kquick commented 5 years ago

Interesting bug; hopefully the 8.8 fix will introduce more noise from the compiler about these.

Thanks for the update!

emilaxelsson commented 5 years ago

After holidays and a lot of testing on different versions (including my packages that depend on Syntactic) I finally got things to work, except on GHC 8.2, unfortunately. See the comment to the instance below.

instance {-# OVERLAPPABLE #-} BindingDomain sym
  where
    prVar _ = Nothing
    prLam _ = Nothing
    renameBind _ a = a
  -- This instance seems to overlap all others on GHC 8.2.2. This leads to
  -- failures in the test suite. Removing the instance and declaring one
  -- instance per type solves the problem. Earlier and later GHC versions don't
  -- have this problem, so I assume it's a bug in 8.2.

I've explicitly disabled GHC 8.2 in the Cabal file.

Thanks for your patch and for getting things going!