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

Remove GHC 8.4 #1762

Merged
merged 2 commits into from
Apr 14, 2021
Merged

Remove GHC 8.4 #1762

merged 2 commits into from
Apr 14, 2021

Conversation

alex-mckenna
Copy link
Contributor

To support using -XQuantifiedConstraints in the partial evaluator,
support for GHC 8.4 is dropped. Conditionally compiled code for
pre GHC 8.4 is removed, and version bounds are bumped to a minimum
of GHC 8.6.

@martijnbastiaan
Copy link
Member

You can remove 8.4 from here too:

GHC_VERSIONS="9.0.1 8.10.3 8.8.4 8.6.5 8.4.4"

To support using -XQuantifiedConstraints in the partial evaluator,
support for GHC 8.4 is dropped. Conditionally compiled code for
pre GHC 8.4 is removed, and version bounds are bumped to a minimum
of GHC 8.6.
#else
U ((shiftL v i) `mod` m)
#endif
else
error ("'shiftL undefined for negative number: " ++ show i)
where
#if MIN_VERSION_base(4,15,0)
m = 1 `naturalShiftL` naturalToWord (natVal (Proxy @n))
sz = naturalToWord (natVal (Proxy @n))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

natToNum @n?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, my feeling is that these internal modules could do with a little spring cleaning in places anyway. Leave it for another PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. I commented because you changed it, that's all :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is approved, feel free to merge 😉

Copy link
Member

@christiaanb christiaanb Apr 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that the generated Core is as efficient if you do decide to switch to natToNum, since natToNum goes through both SNat and Integer, where naturalToWord is basically:

naturalToWord :: Natural -> Word
naturalToWord (Small# w) = W# w
naturalToWord _ = error "too big"

which basically translates to the follow Core

\x -> case x of
  Small# w -> W# w
  _ -> error "too big"

if natToNum translates to that simple case statement, then use it; otherwise keep naturalToWord (natVal (Proxy @n))

Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @leonschoorl can you think of anything else?

@leonschoorl
Copy link
Member

I found a couple more

STYLE.rst Show resolved Hide resolved
@leonschoorl
Copy link
Member

Was surprised to see it still compile with the removal of HsVersions.h
Must be getting it from the host compiler, because it's still used in all newer version:

@alex-mckenna
Copy link
Contributor Author

Was surprised to see it still compile with the removal of HsVersions.h

I was initially surprised when I noticed it was only in the 8.4 dir, then assumed that other people who did the GHC stuff for 8.6, 8.8, 8.10, 9.0 knew what they were doing (given that it still compiled, and therefore works 😉 )

@martijnbastiaan martijnbastiaan merged commit 25d02b2 into master Apr 14, 2021
@martijnbastiaan martijnbastiaan deleted the remove-ghc-8.4 branch April 14, 2021 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants