Opened 6 years ago

Last modified 15 months ago

#8489 new task

clean up dependency and usages handling in interface files

Reported by: errge Owned by: mgsloan
Priority: normal Milestone:
Component: Template Haskell Version: 7.7
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

While reviewing #1480, it was found that there are corner cases that are not easy to handle.

This task is about fixing TH module reification to reify direct import list, not usages. For this we may need to extend the current ModIface.

This task also includes cleaning up the comments and code in related areas, to quote simonpj: "we need more precise commentary on the fields of HscTypes.Dependencies, TcRnTypes.ImportAvails, and mi_usages of ModIface. For example, I wonder whether the mi_usages field could be part of the Dependencies".

Change History (13)

comment:1 Changed 6 years ago by errge

Blocked By: 1480 removed

comment:2 Changed 5 years ago by thomie

I don't know what the status is here. Is it cleaned up? I did find this commit: 50d4cd7afd8fdc3efdc3a50b9e8e606336c1cdee

Author: Simon Peyton Jones <>
Date:   Tue Nov 5 15:17:22 2013 +0000

    More comments on Usage and Dependencies
Last edited 5 years ago by thomie (previous) (diff)

comment:3 Changed 5 years ago by errge

Thomie, unfortunately this is not done yet and that is totally on me :(

The main task here is to "fix TH module reification to reify direct import list, not usages"

So just having more comments is not enought, actual coding (and probably some refactoring) has to happen. On the other hand, I am pretty sure that the better comments will help me to get back up to speed more quickly when I sit down to handle this.

I still plan to work on this, so feel free to leave me as owner unless someone else wants to tackle this before me in which case I am happy to discuss more if questions come up.

Last edited 5 years ago by errge (previous) (diff)

comment:4 Changed 5 years ago by simonpj

A good first step would be to write a precise specification for the change you propose. What will change? Is is just refactoring or will (TH) programmers see a difference? What will the difference be? I have no idea at the moment.

Once the specification is clear, and agreed, the implementation can follow. But the latter should not precede the former.

Simon

comment:5 Changed 5 years ago by errge

Simon, yes of course, it is important to first agree on what we want.

On the other hand, this is a much smaller change than my changes from previous year, so I do not think that we need a whole wiki page again.

Let me put it here what I think the issue is and then you can point it out where I was not precise enough and we can complete this to a "specification".

Take the following program:

{-# LANGUAGE TemplateHaskell #-}

import Language.Haskell.TH
import Language.Haskell.TH.Syntax

$(do let mod = Module (PkgName "binary-0.7.1.0") (ModName "Data.Binary")
     ModuleInfo info <- reifyModule mod
     runIO $ mapM_ print info
     return [])

main = return ()

On my machine, the output is:

Module (PkgName "base") (ModName "Data.Either")
Module (PkgName "base") (ModName "Data.Maybe")
Module (PkgName "base") (ModName "Data.Word")
Module (PkgName "base") (ModName "GHC.Base")
Module (PkgName "base") (ModName "GHC.IO")
Module (PkgName "base") (ModName "GHC.IO.IOMode")
Module (PkgName "base") (ModName "GHC.Word")
Module (PkgName "base") (ModName "Prelude")
Module (PkgName "base") (ModName "System.IO")
Module (PkgName "binary-0.7.1.0") (ModName "Data.Binary.Class")
Module (PkgName "binary-0.7.1.0") (ModName "Data.Binary.Generic")
Module (PkgName "binary-0.7.1.0") (ModName "Data.Binary.Get")
Module (PkgName "binary-0.7.1.0") (ModName "Data.Binary.Get.Internal")
Module (PkgName "binary-0.7.1.0") (ModName "Data.Binary.Put")
Module (PkgName "bytestring-0.10.4.0") (ModName "Data.ByteString")
Module (PkgName "bytestring-0.10.4.0") (ModName "Data.ByteString.Lazy")
Module (PkgName "bytestring-0.10.4.0") (ModName "Data.ByteString.Lazy.Internal")
Module (PkgName "ghc-prim") (ModName "GHC.Types")

And looking at http://hackage.haskell.org/package/binary-0.7.1.0/docs/src/Data-Binary.html it can be seen that we do not import GHC.Types in binary.

So from the programmer point of view, this change should make the ModuleInfo list returned by module reification only contain the actual direct imports.

So this is my goal, before discussing implementation details inside GHC, may I ask if this description is clear enough and if others agree with the goal?

comment:6 Changed 5 years ago by simonpj

Aha I see. So this is really a bug, thus:

  • Template Haskell offers
    reifyModule :: Module -> Q ModuleInfo
    data ModuleInfo = ModuleInfo [Module]
      -- Contains the import list of the module
    
  • The "import list of the module" M presumably means the list of modules that M imports directly. (Clarifying the comment would be good.)
  • But the program above shows that reifyModule reports a much longer list. (NB: A smaller test case would make easier.)

So the specification is simply to make reifyModudule behave as advertised.

Now I understand, it's easy enough to implement, at least for modules in the same package as M. Just pick modules whose Usage has Just _ in its usg_exports field. Sadly, for modules from other pacakges we simply don't record the necessary information in interface files at all. It would not be hard to fix this.

comment:7 Changed 5 years ago by errge

If I remember correctly the difference is not the package boundary, but the compilation unit boundary. So if you compile your modules with different GHC invocations by using -c instead of --make, then you will not have the usg_exports either, am I right?

If that is the case I prefer us to do the right fix and include this information in the interfaces files and do not introduce different behavior depending on compilation technique in the meantime.

comment:8 Changed 5 years ago by simonpj

I believe its the package boundary.

comment:9 Changed 5 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:10 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:11 Changed 4 years ago by thomie

Milestone: 8.0.1

comment:12 Changed 15 months ago by mgsloan

I don't understand how the explicit imports list of a module would be helpful to another module. I've never grokked the usecase for this.

I suggest just changing the documentation to indicate that it is a list of usages rather than imports, to resolve this. How's that sound? Assigning this ticket to myself, to implement via documentation.

Alternatively, this usages information could just get removed. Worth checking if there are uses of it on hackage. I am considering implementing reification of exports (https://ghc.haskell.org/trac/ghc/ticket/10391), which is what I think most people would want in module reification as that would be quite useful.

comment:13 Changed 15 months ago by mgsloan

Owner: changed from errge to mgsloan
Note: See TracTickets for help on using tickets.