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

comp: remove most imports of Data.Generics #722

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thoughtpolice
Copy link
Contributor

99% of the uses of the Data.Generics import from syb was to qualify the names Data and Typeable, but these have long since been available directly in base along with DeriveDataTypeable for many years now. The syb exports are in fact just re-exports from base. Migrate most of the code to just use Data.Data directly instead.

At the same time, this touches up some of the import lists to be a little more consistent but is otherwise functionally identical.

99% of the uses of the `Data.Generics` import from `syb` was to qualify the
names `Data` and `Typeable`, but these have long since been available directly
in `base` along with `DeriveDataTypeable` for many years now. The `syb` exports
are in fact just re-exports from base. Migrate most of the code to just use
`Data.Data` directly instead.

At the same time, this touches up some of the import lists to be a little more
consistent, but is otherwise functionally identical.

Signed-off-by: Austin Seipp <aseipp@pobox.com>
Copy link
Collaborator

@quark17 quark17 left a comment

Choose a reason for hiding this comment

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

Does "long since" include GHC 7.10.3, or will this increase the minimum GHC version? If so, do you know what version?


ordC :: IConInfo a -> Int
-- XXX This definition would be nice, but it imposes a (Data a) context
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you removing this comment, because it's no longer meaningful? If it's still meaningful, I'd leave it in. (I don't offhand understand what it's saying.)


import qualified Data.Generics as Generic
import Data.Graph
import Control.Monad.State(State, evalState, liftIO, get, put)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You reorder a few import lists, and I don't quite understand your choices, but OK.

@@ -55,10 +55,10 @@ module CType(
import Prelude hiding ((<>))
#endif

import Data.Data (Data, Typeable)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you intend a space here?


-- ==============================
-- IConInfo

-- FIXME: syb includes an orphan to work around the fact Handle needs a Data
Copy link
Collaborator

Choose a reason for hiding this comment

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

And the only place we have Handle in a data structure needing Data is here in IConInfo (ICHandle), so you've chosen to put the instance here? Ok.

@Vekhir
Copy link
Contributor

Vekhir commented Sep 24, 2024

Does "long since" include GHC 7.10.3, or will this increase the minimum GHC version? If so, do you know what version?

Data.Data exists since base 4.6.0.1: https://hackage.haskell.org/package/base-4.6.0.1/docs/Data-Data.html#t:Data
That's the oldest I could find. This corresponds to GHC 7.6 or later, so the current bounds are fine.
Not done here, but the DeriveDataTypeable language extension is enabled by default since GHC 7.10 (source). It's mentioned in the OP.

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