Copilot-Language/copilot

`copilot-core`: record field `uTypeType` is unnecessary

Closed this issue · 6 comments

Description

The record field uTypeType, part of Copilot.Core.Type.UType, is never really used within the module or in any other part of Copilot or, as far as we know, by any user of Copilot. As a matter of fact, we cannot use it to obtain the value inside the UType due to the existential it encloses. It also makes the test report lower coverage than they otherwise might, since the record access function is not used by the tests.

This field is not necessary parts of the interface to use Copilot, and not required by Copilot's requirements. Therefore, it would be suitable to remove that definitions from the module.

As per our internal policy of waiting 3 versions from deprecation until a public interface declaration can be removed, those definitions should be deprecated.

Type

  • Bug: unused code included in the implementation.

Additional context

None.

Requester

  • Ivan Perez

Method to check presence of bug

The field is mentioned in the record definition, but making it a non-record datatype, or not exporting that field in the module exports, does not change the behavior and does not otherwise affect Copilot.

The code compiles without errors and behaves in the same way.

Expected result

The following elements are deprecated:

  • Copilot.Core.Type.UType.uTypeType

Desired result

The following elements are deprecated:

  • Copilot.Core.Type.UType.uTypeType

Proposed solution

Deprecate the following functions:

  • Copilot.Core.Type.UType.uTypeType

Further notes

None.

Change Manager: Confirmed that the issue exists.

Technical Lead: Confirmed that the issue should be addressed.

Technical Lead: Issue scheduled for fixing in Copilot 4.1.

Fix assigned to: @ivanperez-keera .

Implementor: Solution implemented, review requested.

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/272931486
    • The solution proposed produces the expected result. Details:
      The following Dockerfile tries to import and use uTypeType, printing "Success" if the import is deprecated, and "Failure" otherwise:
      ENV DEBIAN_FRONTEND=noninteractive
      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 g++ make libgmp3-dev
      RUN apt-get install --yes pkg-config
      
      SHELL ["/bin/bash", "-c"]
      
      RUN ghcup install ghc 9.8.2
      RUN ghcup install cabal 3.2
      RUN ghcup set ghc 9.8.2
      RUN cabal update
      
      SHELL ["/bin/bash", "-c"]
      CMD git clone $REPO && cd $NAME && git checkout $COMMIT && cd .. \
        && cabal v1-sandbox init \
        && cabal v1-install $NAME/copilot-core/ \
        && ! cabal v1-exec -- runhaskell -XGADTs -Wall -Werror <<< 'import Copilot.Core.Type(UType(UType, uTypeType), Type(Int8)); main :: IO (); main = case UType Int8 of; (UType { uTypeType = Int8}) -> putStrLn "Failure"' \
        && 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=2a476637e44df5b1c125fa5c270c300479e0df72" -it copilot-verify-484
      
  • Implementation is documented. Details:
    No need for additional documentation (deprecation).
  • Change history is clear.
  • Commit messages are clear.
  • Changelogs are updated.
  • Examples are updated. Details:
    No updates needed (deprecation).
  • Required version bumps are evaluated. Details:
    Major bump required. Public element being deprecated is an API-breaking change.

Change Manager: Implementation ready to be merged.