-
Notifications
You must be signed in to change notification settings - Fork 146
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
base: main
Are you sure you want to change the base?
comp: remove most imports of Data.Generics
#722
Conversation
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 <[email protected]>
There was a problem hiding this 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 |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
|
99% of the uses of the
Data.Generics
import fromsyb
was to qualify the namesData
andTypeable
, but these have long since been available directly inbase
along withDeriveDataTypeable
for many years now. Thesyb
exports are in fact just re-exports from base. Migrate most of the code to just useData.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.