Skip to content
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

Make it compile with MicroHs #262

Merged
merged 11 commits into from
Nov 17, 2024
Merged

Make it compile with MicroHs #262

merged 11 commits into from
Nov 17, 2024

Conversation

augustss
Copy link
Contributor

Three kinds of changes:

  • MicroHs does not have Generic
  • MicroHs does not have Template Haskell
  • MicroHs does not have pattern synonyms

@AshleyYakeley
Copy link
Member

What's the best way of testing this? I'm worried this will break if it's not tested in CI.

@augustss
Copy link
Contributor Author

I meant to add it to CI, but I forgot to do that. I'll do that soon.

@AshleyYakeley
Copy link
Member

Your CI failed. Looks like a shell arg-quoting issue or something.

@augustss
Copy link
Contributor Author

Yes. It needed a newer mhs version. It's now using stable-2 instead of stable-1.

@AshleyYakeley
Copy link
Member

Can you get CI to run the main test suite?

@augustss
Copy link
Contributor Author

I'm not sure. Does the other CI stuff do that?

@augustss
Copy link
Contributor Author

I think it's a big fiddly to get it to run as part of the test suite.
You need to git clone the mhs repo, build mhs, and then compile.

@AshleyYakeley
Copy link
Member

Ubuntu, MacOS, Windows and FreeBSD CI all run tests. WASM does not, because apparently tasty isn't supported, though it does run the (very small) ShowTime and ShowDefaultTZAbbreviations tests.

@AshleyYakeley
Copy link
Member

Does MicroHs work with cabal?

@augustss
Copy link
Contributor Author

I misunderstood. No, MHS can't run the test suite. It needs more stuff, e.g., tasty.
But I'll see if I can add the simple tests.

@augustss
Copy link
Contributor Author

"Does MicroHs work with Cabal?"
Depends on what you mean.

  • Cabal can build the mhs binary.

  • Cabal cannot use MHS to build libraries nor binaries.

I've made MicroCabal which can use MHS (and GHC) to build stuff.

@augustss
Copy link
Contributor Author

I've found some problems that I have to fix, so don't merge my changes yet.

@AshleyYakeley AshleyYakeley marked this pull request as draft November 11, 2024 13:20
@AshleyYakeley
Copy link
Member

OK, I've converted it to draft. Convert it back when it's ready.

@augustss
Copy link
Contributor Author

I think my changes are ready to be merged.

@AshleyYakeley AshleyYakeley marked this pull request as ready for review November 16, 2024 20:56
@AshleyYakeley
Copy link
Member

I take it ./format-all -b no longer works?

@augustss
Copy link
Contributor Author

I don't know what format-all does, but I can test it. It uses stack, which I don't. But I'll install stack and see what happens.

@AshleyYakeley
Copy link
Member

It runs fourmolu on the source files. However, fourmolu can't handle source files with CPP in them.

@AshleyYakeley
Copy link
Member

Hmm, looks like fourmolu does have CPP support. In any case, don't worry about it.

@AshleyYakeley AshleyYakeley merged commit 44be01a into haskell:master Nov 17, 2024
12 checks passed
@augustss
Copy link
Contributor Author

Thanks!

Btw, I really hate adding CPP, but I don't know any other way. As MicroHaskell gets more features, some of them can be removed again.

@Bodigrim
Copy link
Contributor

Bodigrim commented Nov 17, 2024

(Just passing by)

CPP can be done a little bit better using {-# LANGUAGE StandaloneDeriving #-}: instead of

newtype Month = MkMonth Integer deriving (Eq, Ord, Data, Typeable
#if __GLASGOW_HASKELL__
                                                                 , TH.Lift, Generic
#endif
                                                                                   )

one can write

newtype Month = MkMonth Integer deriving (Eq, Ord, Data, Typeable)
#if __GLASGOW_HASKELL__
deriving instance TH.Lift Month
deriving instance Generic Month
#endif

This way CPP does not appear in the middle of declaration, making it easier for formatters and such simply ignore it.

As for PatternSynonyms, could MicroHS support declaring them (which would be no-op, simply parse and ignore), but keep prohibiting their usage?

@AshleyYakeley
Copy link
Member

Yup, currently working on this.

@AshleyYakeley
Copy link
Member

Yeah the patterns are really annoying. Possibly it would have been better to wait for pattern synonyms support.

@augustss
Copy link
Contributor Author

Adding parsing of pattern synonyms should be pretty easy. And then just ignoring them for now.

@AshleyYakeley
Copy link
Member

Adding parsing of pattern synonyms should be pretty easy. And then just ignoring them for now.

That would be very helpful.

The instance deriving is no problem, I can use @Bodigrim's method or even this:

newtype Month = MkMonth Integer
#if __GLASGOW_HASKELL__
    deriving (Eq, Ord, Data, Typeable, TH.Lift, Generic)
#else
    deriving (Eq, Ord, Data, Typeable)
#endif

@augustss
Copy link
Contributor Author

Actually, I could fake having deriving for TH.Lift and Generic as well. That would make for even fewer changes.
Let me do that.

@augustss
Copy link
Contributor Author

Regarding pattern synonyms. It's easy to just ignore the definitions to avoid CPP. But the synonyms are also frequently used, and those will still need CPP.

@AshleyYakeley
Copy link
Member

Yeah that seems unavoidable, though in some cases I may be able to rewrite them.

Of course, you could just actually implement pattern synonyms in MicroHs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants