[GHC] #8426: one-shot compilation + TH doesn't see instances that is seen in batch mode
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------------------------+---------------------------- Reporter: errge | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Template Haskell | Version: 7.6.3 Keywords: | Operating System: Architecture: Unknown/Multiple | Unknown/Multiple Difficulty: Moderate (less than a day) | Type of failure: Other Blocked By: | Test Case: Related Tickets: #7867 | Blocking: ----------------------------------------------+---------------------------- Haskell prime and Haskell98 both states that instances are always seen if there is any chain of imports leading from the current module to the instance. http://darcs.haskell.org/haskell-prime-report/report/haskell-report- html/modules.html#import-instances The ghc manual contains a discussion why this specification is too performance heavy to näively implement and contains the definition of orphanness and an algorithm to reproduce the specification with better performance. http://www.haskell.org/ghc/docs/7.6.3/html/users_guide/separate- compilation.html#orphan-modules Unfortunately this cleverness breaks template haskell. Namely, if: - `Class.hs` contains a class, - `NonOrphan.hs` contains a data that implements the class, - `Importer.hs` just imports `NonOrphan.hs`, - `Main.hs` imports `Importer.hs`, - `Main.hs` reifies the class; then: - one-shot compilation's reify doesn't see the instance, - batch compilation's reify sees the instance. This ambiguity is shown in the attached tgz. Furthermore if `NonOrphan.hs` is in a separate package, then of course even batch compilation mode wouldn't reify the instance correctly. In that case there is no disambiguity between one-shot and batch, simply both is missing the info. I propose to solve this by enforcing a loading of all the interface files for every import transitively for the current module, when we meet an open type family or class reification request in template haskell's reify handler in `TcSplice.hs`. So I propose to keep the optimization of orphan instances and use it 99.9% of the time. We would only ever go on the quest of reading interface files for every import transitively when the user tries to reify something where we have to return an instance list. This means that TH compilations that reify types will get a little bit slower in exchange of being correct and unambiguous. Compilation time in cases when TH reification is not used will not change. I volunteer to prepare the patch, but would like to hear others first. So, any opinions, alternative ideas? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------+---------------------------------------------- Reporter: errge | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: | Version: 7.7 Template Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less than a day) Unknown/Multiple | Blocked By: Type of failure: Other | Related Tickets: #7867 Test Case: | Blocking: | ----------------------------+---------------------------------------------- Changes (by errge): * version: 7.6.3 => 7.7 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------+---------------------------------------------- Reporter: errge | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: | Version: 7.7 Template Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less than a day) Unknown/Multiple | Blocked By: Type of failure: Other | Related Tickets: #7867 Test Case: | Blocking: | ----------------------------+---------------------------------------------- Comment (by errge): For related work, see also the discussion in: http://ghc.haskell.org/trac/ghc/wiki/TemplateHaskell/Annotations -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------+---------------------------------------------- Reporter: errge | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: | Version: 7.7 Template Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less than a day) Unknown/Multiple | Blocked By: Type of failure: Other | Related Tickets: #7867 Test Case: | Blocking: | ----------------------------+---------------------------------------------- Comment (by simonpj): That seems plausible. But reading the transitive closure of all interface files could be thousands of files! And 90% of the time when reifying a family or class you don't actually want an exhaustive list of all possible instances in all possible modules. A possible alternative is this: remove `[InstanceDec]` from `ClassI` and `FamilyI`. Instead you have to use `qReifyInstances` to find out about instances. Because the latter has a `[Type]` you can use that to know which interface files to load. To me that sounds simpler (delete code rather than add it), without really removing useful functionality. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------+---------------------------------------------- Reporter: errge | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: | Version: 7.7 Template Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less than a day) Unknown/Multiple | Blocked By: Type of failure: Other | Related Tickets: #7867 Test Case: | Blocking: | ----------------------------+---------------------------------------------- Comment (by errge): Yes, the exhaustive list can be big. I agree with your plan of just removing [InstanceDec] from ClassI and FamilyI, fixing the ambiguity. But actually I need the functionality of looking up instances without types. So what if we somehow expose that functionality directly? Meaning that only those users will pay the cost who explicitly want to do that. I see two options: - (clean) adding a new Q monad method for this, - (a bit hacky) specifying that qReifyInstances with an *empty* list parameter does that. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------+---------------------------------------------- Reporter: errge | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: | Version: 7.7 Template Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less than a day) Unknown/Multiple | Blocked By: Type of failure: Other | Related Tickets: #7867 Test Case: | Blocking: | ----------------------------+---------------------------------------------- Comment (by errge): Over the weekend I implemented option 1. So, what the patch currently does is the following: - as Simon suggested, get rid of instance reification when reifying a class or type family, because they can't be made unambiguous, - add a new Q monad statement to get the instances from all of our dependencies: this may be slow, but never called explicitly, so doesn't add accidental slowness, - the patch is careful that even if you call the slow transitive instance search, we don't pollute the EPS map, so future EPS lookups won't get slower because of bigger map size. I also attached the necessary one liner change in dph and the testsuite updates. Ran full validate (not just fast), the attached testsuite patch fixes everything what this change causes. On top of this it's easy to implement the controversial "module listing" functionality from #8398 cleanly. We just load transitively in every interface file and then return all of our dependencies. That's well defined and not compiler implementation dependent. Please review. Thanks. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------+---------------------------------------------- Reporter: errge | Owner: Type: bug | Status: patch Priority: normal | Milestone: 7.8.1 Component: | Version: 7.7 Template Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less than a day) Unknown/Multiple | Blocked By: Type of failure: Other | Related Tickets: #7867 Test Case: | Blocking: 8398 | ----------------------------+---------------------------------------------- Changes (by errge): * status: new => patch * blocking: => 8398 * milestone: => 7.8.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode ----------------------------+---------------------------------------------- Reporter: errge | Owner: Type: bug | Status: patch Priority: normal | Milestone: 7.10.1 Component: | Version: 7.7 Template Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less than a day) Unknown/Multiple | Blocked By: Type of failure: Other | Related Tickets: #7867 Test Case: | Blocking: | ----------------------------+---------------------------------------------- Changes (by errge): * milestone: 7.8.1 => 7.10.1 Comment: The provided patch changes the TH API and the testsuite needs to be updated. At this point it seems more meaningful to target 7.10.1 with this change. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode -------------------------------------+------------------------------------- Reporter: errge | Owner: Type: bug | Status: infoneeded Priority: normal | Milestone: 7.12.1 Component: Template | Version: 7.7 Haskell | Keywords: Resolution: | Architecture: Unknown/Multiple Operating System: | Difficulty: Moderate (less Unknown/Multiple | than a day) Type of failure: Other | Blocked By: Test Case: | Related Tickets: #7867 Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- Changes (by thoughtpolice): * status: patch => infoneeded * milestone: 7.10.1 => 7.12.1 Comment: Gergley, sorry for dropping this patch on the floor. As you can tell since last year a lot has changed! Can you please rebase these patches? I also vaguely remember you wrote a wiki page about what they all addressed. Thanks! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode -------------------------------------+------------------------------------- Reporter: errge | Owner: Type: bug | Status: infoneeded Priority: normal | Milestone: 7.12.1 Component: Template Haskell | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Other | Unknown/Multiple Blocked By: | Test Case: Related Tickets: #7867 | Blocking: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by ezyang): The patches are pretty short; I think the real reason this ticket is stalled is because simonpj never signed off on errge's suggested implementation strategy; and in the meantime, it seems this bug has been routed around. Perhaps we should just apply simonpj's original suggestion (remove `[InstanceDec]` from `ClassI` and `FamilyI`) and call this bug fixed. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode -------------------------------------+------------------------------------- Reporter: errge | Owner: Type: bug | Status: infoneeded Priority: normal | Milestone: 7.12.1 Component: Template Haskell | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Other | Unknown/Multiple Blocked By: | Test Case: Related Tickets: #7867 | Blocking: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by errge): Replying to [comment:10 ezyang]:
The patches are pretty short; I think the real reason this ticket is stalled is because simonpj never signed off on errge's suggested implementation strategy; and in the meantime, it seems this bug has been routed around. Perhaps we should just apply simonpj's original suggestion (remove `[InstanceDec]` from `ClassI` and `FamilyI`) and call this bug fixed.
No, I would like to strongly ask you not to do that. As I mentioned before in the bug report, I depend on the functionality to look up instance declarations for classes. If this info were to be removed, then there would be no way for me to continue to support the HFlags library after 7.12. As can be seen in the history of the ticket in comment 5 I have a patch which keeps the functionality in a very careful way (doesn't cause any accidental slowness for people who don't want to use it). It's also easy to understand from the user's point of view: looking up instances is expensive so you have to call it explicitly if you ever want it. I'm prepared to rebase that patch and fix up the test suite, if I get some agreement from a maintainer with actual commit rights that the idea is acceptable. Of course, this is not a blank cheque, I would still have to provide quality code, but not willing to work a lot again on this just so that you guys drop this without ever replying or discussing with me for 14 months. I remember prioritizing this work over my other commitments exactly so it doesn't stall and keeps moving, but no one cared. Sorry for the harsh words, I can of course totally understand that for everyone else the contribution time is already thin and very valuable; this is why what I'm looking for is an agreement on goals and designs before anyone (you or me) has to invest serious time and effort on either side. I'm also open to other implementation ideas, but I seriously depend on this feature so would like to ask you guys to keep it on in some form. Thanks and sorry for the long comment! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode -------------------------------------+------------------------------------- Reporter: errge | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Template Haskell | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: #7867 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * cc: goldfire (added) Comment: errge, it seems that this was dropped yet again; I'm sorry that this process has been so messy. Would you like to rebase your patches and put them up on Phabricator? There we can do a proper review. I'm also cc'ing goldfire, who is our Template Haskell czar. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#8426: one-shot compilation + TH doesn't see instances that is seen in batch mode -------------------------------------+------------------------------------- Reporter: errge | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Template Haskell | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: #7867 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by spacekitteh): Does this still occur? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8426#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC