Updates Haskell LTS version to 18.28#977
Conversation
The Network module is removed, and has been replaced with a combination of the Network.Socket module, and functions from TextViaSockets. Exception handling is removed from the haskeline library. It is replaced with the exceptions library instead.
The monoid typeclass has split into Monoid and Semigroup, making old implementations of Monoid incorrect. This is fixed by moving the implementation of mappend to the implementation of <> in a new instance of semigroup
Mostly removes redundant monad imports, and replaces the Network library with Network.Socket.
Replaces the removed Network module with parts from Network.Socket, and parts from TextOverSockets. Definately needs testing.
Add Semigroup instance, and replace some calls to Network module with calls to Network.Socket module.
…solver, and adds notes to each file pointing to the need to keep the other files updated if things change again in the future.
tests build now, but they do not execute because they expect the torxakis executable to be on the path, wich messes with global instalations.
Try to standardize the working directory for all tests to the main git folder. Some tests were running in the sqatt folder, while others were not. This makes using relative paths to the torxakis executable impossible. Currently the benchmark sanity tests are still failing nondeterministically.
parallel test execution was giving nondeterministic errors.
Errors seemed to not be related to the body of the tests, as replacing all tests with 1 `shouldbe` 10 still crashed.
This only seemed to happen when at least 3 testsets where given as an argument to the testExampleSets function, but more frequently with more testsets being used.
observed error:
$ stack test sqatt:benchmarks-sanity-test
sqatt> test (suite: benchmarks-sanity-test)
sqatt> Test suite benchmarks-sanity-test failed
Error: [S-7282]
Stack failed to execute the build plan.
While executing the build plan, Stack encountered the error:
Error: [S-1995]
Test suite failure for package sqatt-0.1
benchmarks-sanity-test: exited with: ExitFailure (-1073741675)
expected behaviour: something like
$ stack test sqatt:benchmarks-sanity-test
sqatt> test (suite: benchmarks-sanity-test)
Debug set 1
dummy test. FAILED [1]
Debug set 1
dummy test. FAILED [2]
Debug set 1
dummy test. FAILED [3]
Failures:
src\Sqatt.hs:497:46:
1) Debug set 1 dummy test.
expected: 10
but got: 1
To rerun use: --match "/Debug set 1/dummy test./"
src\Sqatt.hs:497:46:
2) Debug set 1 dummy test.
expected: 10
but got: 1
To rerun use: --match "/Debug set 1/dummy test./"
src\Sqatt.hs:497:46:
3) Debug set 1 dummy test.
expected: 10
but got: 1
To rerun use: --match "/Debug set 1/dummy test./"
Randomized with seed 1632928150
Finished in 0.0006 seconds
3 examples, 3 failures
Test removed as it does not terminate, tested in latest develop, v0.9 and v0.6,
Mostly removed redundant imports and error cases. Temporarily adds Unique as extra dependency, because replacing the depriciated package is probably not worth it if the module in which it is used (txs-compiler) is planned to be phased out.
Default is relative path starting in root git directory. Relative paths starting in sqatt directory need to use explicit insqatt modifier.
Update LTS version for newer ghc compiler
| extra-deps: | ||
| - git: https://github.com/TorXakis/text-via-sockets.git | ||
| commit: e3228cd0407ec0d7991a544e154ea2def184fcae | ||
| - git: https://github.com/gijsvcuyck/text-via-sockets.git |
There was a problem hiding this comment.
Why not merge this fork back first? Then you won't need to change this reference.
There was a problem hiding this comment.
Why not merge this fork back first? Then you won't need to change this reference.
I just made both pull requests at the same time, since I figured it might take a while before anything got merged. The other pull request is here.
It would indeed be better to merge the the test-via-sockets changes first, but it also would not be that hard to change the reference again later.
| hexpat: | ||
| bundle: true | ||
| text: | ||
| integer-simple: true |
There was a problem hiding this comment.
Indeed integer-simple is no longer needed as Haskell is change to use it.
There was a problem hiding this comment.
When I have time, I will try to remove all mentions of integer-simple and rerun the tests.
There was a problem hiding this comment.
I did some googling, and the available information is rather vague again, but it looks like the change in haskell to move away from integer-simple happened around ghc 9, and this pr moves the ghc version up to ghc 8.10.7.
I also heard that integer-simple was originally used for licensing reasons, so I cannot access if it is ok to move away from it myself. As this seems a separate issue from the rest of the pr, I don't think it should be changed here.
The above flag change is separate from this as the flag itself no longer exists. I could not find any documentation on when or why this changed or if it needed to be replaced with something, but removing it did not break any of the tests at least.
| (view -> Vconcat _) -> idsInSubExps | ||
| (view -> Vstrinre _ _) -> idsInSubExps | ||
| (view -> Vpredef _ fid _) -> Set.insert (TxsDefs.IdFunc fid) idsInSubExps | ||
| _ -> error ("GetValExprIds.searchVisitor not defined for " ++ show expr ++ "!") |
There was a problem hiding this comment.
Why did you remove these error messages to the user?
LPE doesn't support all construct (e.g. not dynamic process creation),
user should be informed when he is using LPE incorrectly.
There was a problem hiding this comment.
I removed the default case here, and in several other places, because ghc was giving warnings about duplicate cases. According to ghc, all possible cases of expr were already covered, and so the default case was unnecessary. Since warnings are treated as errors, I had to remove the default case to get it to compile.
I double checked, and the case distinction here is over a value of type ValExprView VarId, and all possible constructors of this type (TorXakis\sys\valexpr\src\ValExprDefs.hs line51) are indeed being covered here.
| , txsServerArgs = ["--smt-solver", "cvc4"] | ||
| , expectedResult = Pass | ||
| } | ||
| -- cvc4 not currently supported. |
There was a problem hiding this comment.
You could use cvc5 instead
There was a problem hiding this comment.
I tried, but the interface changed, and I did not want to put time into fixing features that were already broken and I am not going to use. See #961. This would be something for a separate pull request anyway I think.
pjljvandelaar
left a comment
pjljvandelaar
left a comment
There was a problem hiding this comment.
Indeed don't address all problems at once!
I assume for all not addressed problems new issues are made (and linked to this review).
All the tests still pass without it, and the original licensing issue seems to have been resolved.
4 participants
Updates the LTS version used by the project, allowing the use of newer libraries, and the haskell-language-server implementation of the LSP for IDE support while developing.