#15526 closed task (fixed)

Explain or remove mystery import in Unsafe.Coerce

Reported by: dfeuer Owned by:
Priority: normal Milestone: 8.8.1
Component: Core Libraries Version: 8.4.3
Keywords: newcomer Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Other Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D5092
Wiki Page:

Description

Unsafe.Coerce has the line

import GHC.Integer () -- for build ordering

It is not at all clear to me why build ordering demands that GHC.Integer be compiled before Unsafe.Coerce when the latter uses no numeric literals, no arithmetic, and no numeric types. If there's a real reason for this, the comment should be expanded. Otherwise, the import should be removed.

Change History (8)

comment:1 Changed 14 months ago by RyanGlScott

That comment is indeed cryptic, but I bet there is a good reason for this. There is a [Note [Depend on GHC.Integer] in GHC.Base which explains why you often need empty GHC.Integer imports:

Note [Depend on GHC.Integer]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The Integer type is special because TidyPgm uses
GHC.Integer.Type.mkInteger to construct Integer literal values
Currently it reads the interface file whether or not the current
module *has* any Integer literals, so it's important that
GHC.Integer.Type (in package integer-gmp or integer-simple) is
compiled before any other module.  (There's a hack in GHC to disable
this for packages ghc-prim, integer-gmp, integer-simple, which aren't
allowed to contain any Integer literals.)

Likewise we implicitly need Integer when deriving things like Eq
instances.

The danger is that if the build system doesn't know about the dependency
on Integer, it'll compile some base module before GHC.Integer.Type,
resulting in:
  Failed to load interface for ‘GHC.Integer.Type’
    There are files missing in the ‘integer-gmp’ package,

Bottom line: we make GHC.Base depend on GHC.Integer; and everything
else either depends on GHC.Base, or does not have NoImplicitPrelude
(and hence depends on Prelude).

Moreover, this workaround is still actively being used—a commit as recent as April 2018 (see 54acfbbf64f5fcb108836412e9be0cfabf0d7801) had to introduce additional GHC.Integer () imports to resolve build ordering issues. In that commit, they followed import GHC.Integer () with -- see Note [Depend upon GHC.Integer] in libraries/base/GHC/Base.hs. Would you consider this ticket fixed if that comment were copied over to Unsafe.Coerce as well?

comment:2 Changed 14 months ago by dfeuer

Thanks, Ryan. Yes, I think that would be sufficient.

comment:3 Changed 14 months ago by RyanGlScott

There are a handful of other places that have similarly vague comments (without references to Notes), such as GHC.Maybe, GHC.Stack.Types, and GHC.Err.

comment:4 Changed 14 months ago by RyanGlScott

Keywords: newcomer added

comment:5 Changed 14 months ago by ckoparkar

Differential Rev(s): Phab:D5092

comment:6 Changed 14 months ago by RyanGlScott

Status: newpatch

comment:7 Changed 14 months ago by Krzysztof Gogolewski <krz.gogolewski@…>

In a811d93/ghc:

base: Add references to Notes for certain special imports

Summary:
Modules like GHC.Integer, GHC.Natural etc. are special and sometimes
have to be imported just to resolve build ordering issues. It's useful
to refer to the appropriate Notes at such import sites.

Test Plan: Read it.

Reviewers: RyanGlScott, bgamari, hvr, simonpj

Reviewed By: RyanGlScott, simonpj

Subscribers: simonpj, rwbarton, carter

GHC Trac Issues: #15526

Differential Revision: https://phabricator.haskell.org/D5092

comment:8 Changed 14 months ago by monoidal

Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.