Skip to content

copilot-language: test compile error #469

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
swt2c opened this issue Nov 14, 2023 · 16 comments
Closed

copilot-language: test compile error #469

swt2c opened this issue Nov 14, 2023 · 16 comments
Assignees
Labels
CR:Status:Closed Admin only: Change request that has been completed CR:Type:Bug Admin only: Change request pertaining to error detected
Milestone

Comments

@swt2c
Copy link

swt2c commented Nov 14, 2023

While upgrading to copilot 3.17, we noticed that copilot-language's tests fail to compile with the below error:

Building test suite 'unit-tests' for copilot-language-3.17..
[1 of 3] Compiling Test.Extra       ( tests/Test/Extra.hs, dist-ghc/build/unit-tests/unit-tests-tmp/Test/Extra.o )
[2 of 3] Compiling Test.Copilot.Language.Reify ( tests/Test/Copilot/Language/Reify.hs, dist-ghc/build/unit-tests/unit-tests-tmp/Test/Copilot/Language/Reify.o )

tests/Test/Copilot/Language/Reify.hs:735:16: error:
    • Could not deduce (Copilot.Core.Type.Struct t)
        arising from a superclass required to satisfy ‘Typed t’,
        arising from a use of ‘observer’
      from the context: (Read t, Eq t, Typed t, Arbitrary t)
        bound by a pattern with constructor:
                   SemanticsP :: forall t.
                                 (Typeable t, Read t, Eq t, Show t, Typed t, Arbitrary t) =>
                                 (Stream t, [t]) -> SemanticsP,
                 in an equation for ‘checkSemanticsP’
        at tests/Test/Copilot/Language/Reify.hs:733:33-59
      Possible fix:
        add (Copilot.Core.Type.Struct t) to the context of
          the data constructor ‘SemanticsP’
    • In the expression: observer testObserverName expr
      In an equation for ‘spec’: spec = observer testObserverName expr
      In the expression:
        do let spec = observer testObserverName expr
           llSpec <- reify spec
           let trace = eval Haskell steps llSpec
           let expectation = take steps exprList
           ....
    |
735 |     let spec = observer testObserverName expr
    |                ^^^^^^^^

Unsure if this is possibly related to GHC 9.4 updates, or some other package updates in Debian?

Full compile logs here (for amd64): https://buildd.debian.org/status/fetch.php?pkg=haskell-copilot-language&arch=amd64&ver=3.17-1&stamp=1699932749&raw=0

Let me know if I can provide any additional info.

@ivanperez-keera
Copy link
Member

Thanks a lot! Will review it and get back to you.

@ivanperez-keera ivanperez-keera changed the title copilot-language: test compile error copilot-language: test compile error Nov 14, 2023
@swt2c
Copy link
Author

swt2c commented Nov 15, 2023

Are you able to reproduce this one locally, or do you need any more info from me on it?

@ivanperez-keera
Copy link
Member

ivanperez-keera commented Nov 15, 2023

I'm trying to reproduce it in a docker container with GHC 9.4.7 and Cabal 3.8. Cabal is giving me headaches: haskell/cabal#9444.

@ivanperez-keera
Copy link
Member

I managed to reproduce the problem.

@swt2c
Copy link
Author

swt2c commented Nov 27, 2023

Hey Ivan, just checking in to see if this was looking relatively easy to fix, or whether it might take a bit to sort out. I think this is the last (known) blocker in getting copilot updated in Debian. Obviously not the most urgent issue in the world, though. :-)

@ivanperez-keera
Copy link
Member

ivanperez-keera commented Nov 27, 2023

Hi. I'm hoping we'll be able to fix this soon. I don't know if I'll be able to find time this week but I'll do everything I can. If not, I'll prioritize fixing this by next week.

And thanks for following up on this! Having Copilot in Debian is a big deal for us and your support to make that happen is invaluable :)

@ivanperez-keera
Copy link
Member

ivanperez-keera commented Nov 28, 2023

@swt2c I was able to find the solution. I'll try to push this soon.

If you want to backport this change to Copilot 3.17 on Debian, here's the diff:

diff --git a/copilot-language/tests/Test/Copilot/Language/Reify.hs b/copilot-language/tests/Test/Copilot/Language/Reify.hs
index e056d946..d724df73 100644
--- a/copilot-language/tests/Test/Copilot/Language/Reify.hs
+++ b/copilot-language/tests/Test/Copilot/Language/Reify.hs
@@ -31,7 +31,7 @@ import qualified Copilot.Language.Operators.Integral as Copilot
 import qualified Copilot.Language.Operators.Mux      as Copilot
 import qualified Copilot.Language.Operators.Ord      as Copilot
 import           Copilot.Language.Reify              (reify)
-import           Copilot.Language.Spec               (observer)
+import           Copilot.Language.Spec               (Spec, observer)
 import           Copilot.Language.Stream             (Stream)
 import qualified Copilot.Language.Stream             as Copilot
 
@@ -732,7 +732,8 @@ semanticsShowK steps (SemanticsP (expr, exprList)) =
 checkSemanticsP :: Int -> [a] -> SemanticsP -> IO Bool
 checkSemanticsP steps _streams (SemanticsP (expr, exprList)) = do
     -- Spec with just one observer of one expression.
-    let spec = observer testObserverName expr
+    let spec :: Spec
+        spec = observer testObserverName expr
 
     -- Reified stream (low-level)
     llSpec <- reify spec

@swt2c
Copy link
Author

swt2c commented Nov 28, 2023

Thanks @ivanperez-keera I'll try putting that patch in tonight.

@ivanperez-keera
Copy link
Member

ivanperez-keera commented Nov 28, 2023

Description

copilot-language's tests fail to compile with GHC 9.4. Prior versions made arbitrary decisions during type interfence, and the new type checker will simply reject such programs.

Type

  • Bug: test fails to compile.

Additional context

None.

Requester

  • Scott Talbert (Debian Developer & Debian Haskell Group)

Method to check presence of bug

Compiling copilot-language's tests with GHC 9.4.7 results in an error message:

# cabal test .
Build profile: -w ghc-9.4.7 -O1
In order, the following will be built (use -v for more details):
 - copilot-language-3.17 (test:unit-tests) (first run)
Preprocessing test suite 'unit-tests' for copilot-language-3.17..
Building test suite 'unit-tests' for copilot-language-3.17..
[2 of 3] Compiling Test.Copilot.Language.Reify ( tests/Test/Copilot/Language/Reify.hs, /root/copilot/dist-newstyle/build/x86_64-linux/ghc-9.4.7/copilot-language-3.17/t/unit-tests/build/unit-tests/unit-tests-tmp/Test/Copilot/Language/Reify.o )

tests/Test/Copilot/Language/Reify.hs:737:16: error:
    * Could not deduce (Copilot.Core.Type.Struct t)
        arising from a superclass required to satisfy `Typed t',
        arising from a use of `observer'
      from the context: (Read t, Eq t, Typed t, Arbitrary t)
        bound by a pattern with constructor:
                   SemanticsP :: forall t.
                                 (Typeable t, Read t, Eq t, Show t, Typed t, Arbitrary t) =>
                                 ((Typeable t, Read t, Eq t, Show t, Typed t, Arbitrary t) =>
                                  (Stream t, [t]))
                                 -> SemanticsP,
                 in an equation for `checkSemanticsP'
        at tests/Test/Copilot/Language/Reify.hs:734:33-59
      Possible fix:
        add (Copilot.Core.Type.Struct t) to the context of
          the data constructor `SemanticsP'
    * In the expression: observer testObserverName expr
      In an equation for `spec': spec = observer testObserverName expr
      In the expression:
        do let spec = observer testObserverName expr
           llSpec <- reify spec
           let trace = eval Haskell steps llSpec
           let expectation = take steps exprList
           ....
    |
737 |         spec = observer testObserverName expr

Expected result

The above tests should compile without errors.

Desired result

The above tests should compile without errors.

Proposed solution

Add type annotations in checkSemanticsP.

Further notes

None.

@ivanperez-keera ivanperez-keera added CR:Status:Initiated Admin only: Change request that has been initiated CR:Type:Bug Admin only: Change request pertaining to error detected labels Nov 28, 2023
@ivanperez-keera
Copy link
Member

Change Manager: Confirmed that the issue manifests with GHC 9.4.

@ivanperez-keera ivanperez-keera added CR:Status:Confirmed Admin only: Change request that has been acknowledged by the change manager and removed CR:Status:Initiated Admin only: Change request that has been initiated labels Nov 28, 2023
@ivanperez-keera
Copy link
Member

Technical Lead: Confirmed that the issue should be addressed.

@ivanperez-keera ivanperez-keera added CR:Status:Accepted Admin only: Change request accepted by technical lead and removed CR:Status:Confirmed Admin only: Change request that has been acknowledged by the change manager labels Nov 28, 2023
@swt2c
Copy link
Author

swt2c commented Nov 30, 2023

Thanks @ivanperez-keera, I can confirm this patch gets copilot back to a green board in Debian (ignore mips64el). :-)

image

@ivanperez-keera
Copy link
Member

Technical Lead: Bug scheduled for fixing in Copilot 3.18.

Fix assigned to: @ivanperez-keera.

@ivanperez-keera ivanperez-keera added CR:Status:Scheduled Admin only: Change requested scheduled and removed CR:Status:Accepted Admin only: Change request accepted by technical lead labels Dec 12, 2023
@ivanperez-keera ivanperez-keera added this to the 3.18 milestone Dec 12, 2023
@ivanperez-keera ivanperez-keera self-assigned this Dec 12, 2023
@ivanperez-keera ivanperez-keera added CR:Status:Implementation Admin only: Change request that is currently being implemented and removed CR:Status:Scheduled Admin only: Change requested scheduled labels Dec 17, 2023
ivanperez-keera added a commit to ivanperez-keera/copilot that referenced this issue Dec 17, 2023
…pilot-Language#469.

copilot-language's tests fail to compile with GHC 9.4. Prior versions
made some arbitrary decisions during type interfence, and the new type
checker will simply reject such programs.

This commit modifies a function in the tests that uses an existential
type, by adding an explicit type signature. Doing so helps GHC versions
after 9.4 typecheck the code.
ivanperez-keera added a commit to ivanperez-keera/copilot that referenced this issue Dec 17, 2023
ivanperez-keera added a commit to ivanperez-keera/copilot that referenced this issue Dec 17, 2023
…Refs Copilot-Language#469.

copilot-language's tests fail to compile with GHC 9.4. Prior versions
made some arbitrary decisions during type interfence, and the new type
checker will simply reject such programs.

This commit modifies a function in the tests that uses an existential
type, by adding an explicit type signature. Doing so helps GHC versions
after 9.4 typecheck the code.
ivanperez-keera added a commit to ivanperez-keera/copilot that referenced this issue Dec 17, 2023
@ivanperez-keera
Copy link
Member

Implementor: Solution implemented, review requested.

@ivanperez-keera ivanperez-keera added CR:Status:Verification Admin only: Change request that is currently being verified and removed CR:Status:Implementation Admin only: Change request that is currently being implemented labels Dec 17, 2023
@ivanperez-keera
Copy link
Member

Change Manager: Verified that:

  • Solution is implemented:
    • The code proposed compiles and passes all tests. Details:
      Build log: https://app.travis-ci.com/github/Copilot-Language/copilot/builds/267903584
    • The solution proposed produces the expected result. Details:
      The following Dockerfile checks that copilot-language compiles with GHC 9.4, in which case it prints the message Success:
      FROM ubuntu:focal
      
      RUN apt-get update
      
      RUN apt-get install --yes libz-dev
      RUN apt-get install --yes git
      
      RUN apt-get install --yes wget
      RUN mkdir -p $HOME/.ghcup/bin
      RUN wget https://downloads.haskell.org/~ghcup/0.1.19.2/x86_64-linux-ghcup-0.1.19.2 -O $HOME/.ghcup/bin/ghcup
      
      RUN chmod a+x $HOME/.ghcup/bin/ghcup
      ENV PATH=$PATH:/root/.ghcup/bin/
      ENV PATH=$PATH:/root/.cabal/bin/
      RUN apt-get install --yes curl
      RUN apt-get install --yes gcc make libgmp3-dev g++
      
      SHELL ["/bin/bash", "-c"]
      
      RUN ghcup install ghc 9.4.7
      RUN ghcup install cabal 3.8
      RUN ghcup set ghc 9.4.7
      RUN cabal update
      
      CMD git clone $REPO && \
          cd $NAME && \
          git checkout $COMMIT && \
          cd copilot-language && \
          cabal test . && \
          echo Success
      Command (substitute variables based on new path after merge):
      docker run -e "REPO=https://github.com/ivanperez-keera/copilot" -e "NAME=copilot" -e "COMMIT=e25c7afaf743a5fdc7a9638516f59b006383bef0" -it copilot-verify-469
      
  • Implementation is documented. Details:
    Internal change; no changes to the documentation are needed, and a comment is added to ensure that future revisions do not remove the change accidentally.
  • Change history is clear.
  • Commit messages are clear.
  • Changelogs are updated.
  • Examples are updated. Details:
    No updates needed.
  • Required version bumps are evaluated. Details:
    Bump not needed (internal change to tests; does not affect API).

@ivanperez-keera
Copy link
Member

Change Manager: Implementation ready to be merged.

@ivanperez-keera ivanperez-keera added CR:Status:Closed Admin only: Change request that has been completed and removed CR:Status:Verification Admin only: Change request that is currently being verified labels Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR:Status:Closed Admin only: Change request that has been completed CR:Type:Bug Admin only: Change request pertaining to error detected
Development

No branches or pull requests

2 participants