Ticket #2362 (closed feature request: fixed)

Opened 4 years ago

Last modified 19 months ago

allow full import syntax in GHCi

Reported by: Isaac Dupree Owned by: igloo
Priority: high Milestone: 7.0.1
Component: Compiler Version: 6.8.2
Keywords: ghci, import Cc: rturk@…, jcpetruzza@…, gwern0@…, cgibbard@…, nicolas.pouillard@…, rendel@…, mad.one@…, mjm2002@…, ghci@…, johan.tibell@…, vogt.adam@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

for example, GHCi currently allows "import Data.Map" but not "import Data.Map as Map" or (to demonstrate a fuller range of import syntax) "import qualified Prelude as P hiding (mplus)". (In GHCi, you'd probably only be interested in hiding/qualifying something like Data.Map that made it harder for you to use Prelude functions due to actual ambiguity -- or vice versa)

see  http://article.gmane.org/gmane.comp.lang.haskell.cafe/41272

(P.S. I would also like everything in a module, such as data declarations, to work in GHCi also, but I suspect that's a more complicated feature request?)

Attachments

2362.patch Download (78.4 KB) - added by SamAnklesaria 20 months ago.
2362tests.patch Download (65.2 KB) - added by SamAnklesaria 20 months ago.

Change History

  Changed 4 years ago by guest

  • cc ghci@… added
  • keywords ghci, import added
  • os changed from Unknown to Multiple
  • architecture changed from Unknown to Multiple

Currently you can do this using a helper module, which does the required imports.

module Import where

import qualified Data.Map as Map
*Prelude> :load Import.hs
*Import> Map.insert ...

Of course, doing this in GHCi would be nicer.

  Changed 4 years ago by Isaac Dupree

perhaps somewhat related to #2345

  Changed 4 years ago by igloo

  • difficulty set to Unknown
  • milestone set to 6.10 branch

  Changed 3 years ago by simonmar

  • architecture changed from Multiple to Unknown/Multiple

  Changed 3 years ago by simonmar

  • os changed from Multiple to Unknown/Multiple

  Changed 3 years ago by thoughtpolice

Given the example that :load can load up qualified names as seen above, I don't think this would be too hard to add; if this is to be done, the GHC API seems like the best place to make the change, so ghc-api clients can use it too (like lambdabot.)

Since everything is basically wrapped in a do block, should we just extend that to allow full haskell98-style imports? e.g.

*Prelude> import Data.ByteString.Char8 (pack)
*Prelude Data.ByteString.Char8> import qualified Data.ByteString.Char8 as B
*Prelude Data.ByteString.Char8 B> let x = pack "hi"
*Prelude Data.ByteString.Char8 B> B.length x
2
*Prelude Data.ByteString.Char8 B>

Or, should we only allow GHCi to import things this way? Perhaps by just making an :import command or extending :module to allow full import syntax? Something like,

*Prelude>:m +qualified Data.Text as T
*Prelude T>:t T.null
T.Text
*Prelude T>:m-T
*Prelude>

Trac #1895 seems to indicate that several people want this feature, so I'm wondering if anybody has any objections/sees any problems, or has a better idea? I'd be willing to try and work on this for 6.12 if we can get an idea of what we want.

  Changed 3 years ago by thoughtpolice

  • cc mad.one@… added

  Changed 3 years ago by simonmar

I think it should be part of the GHC API, as a generalisation of API used by :module (setContext). Perhaps setContext should take a [ImportDecl], and we'd also need a way to parse a String into an ImportDecl. We also need to think about how to handle import *M.

For the implementation itself, a good place to start seems to be RnNames.rnImports, which returns a GlobalRdrEnv - exactly what we need to put in the InteractiveContext.

  Changed 3 years ago by igloo

  • priority changed from normal to high
  • milestone changed from 6.10 branch to 6.12.1

See also #1895

  Changed 3 years ago by simonmar

related: #3217

  Changed 3 years ago by simonmar

  • cc rturk@…, jcpetruzza@…, gwern0@…, cgibbard@…, nicolas.pouillard@…, rendel@…, mjm2002@… added

merging with #1895, as this ticket will subsume it.

  Changed 3 years ago by tibbe

  • cc johan.tibell@… added

  Changed 2 years ago by simonmar

  • milestone changed from 6.12.1 to 6.14 branch

Not going to happen for 6.12.1.

  Changed 22 months ago by aavogt

  • cc vogt.adam@… added
  • failure set to None/Unknown

  Changed 22 months ago by igloo

  • milestone changed from 6.14 branch to 6.14.1

  Changed 20 months ago by SamAnklesaria

  • owner set to SamAnklesaria

  Changed 20 months ago by SamAnklesaria

  • owner changed from SamAnklesaria to igloo

My patch separates ghci 'import' syntax from ':module' syntax: while ':module' behaves the same way it always has (not allowing full import syntax), 'import' now acts like the full haskell98 'import'. This means, however, that starred module name imports (that is, full top levels) are now not supported by 'import'. With ':module' still supporting this, though, I don't think this is a problem.

Because import commands are represented by ImportDecls?, I modified the "ic_exports" field of InteractiveContext? to take both modules and their corresponding import declarations, if any. Along with this change comes a modification of getContext, which must use the different "ic_exports" type.

  Changed 20 months ago by simonmar

Many thanks for the patch! I've done a patch review, there are a few things that need fixing, but nothing fatal.

 -> throwOneError (mkPlainErrMsg noSrcSpan (ptext (sLit "No module speficied for import")))

typo in the error message, and perhaps the message should say something like "parse error in import declaration"?

+                                       -- 'ic_toplev_scope', 'ic_exports' and 'ic_imports'

did that sneak in by mistake? I don't see ic_imports anywhere

+getImportDecl :: GhcMonad m => String ->  m (ImportDecl RdrName)
+getImportDecl expr = withSession $ \hsc_env -> hscImport hsc_env ("import"++expr)

Did you really want "import"++expr, with no space between? I'd drop the "import"++ bit entirely, I think, and call the function parseImportDecl (we already have parseName, which is similar).

- | ["import", mod] <- words stmt    = keepGoing' setContext ('+':mod)
+ | ('i':'m':'p':'o':'r':'t':mod) <- stmt    = keepGoing' (importContext True) mod

check for a space after "import", otherwise you'll catch "imported" etc.

What happens if the user imports the same module twice, with different import lists? They should accumulate, as in a source file, right? Does it work that way? What happens if you import a module multiple times with "import", and then say ":m -M"?

In setContext and remembered_ctx it feels to me like an import decl should be a different kind of CtxtCmd, rather than dealing with them separately at a higher level. I'm not sure about the interactions of having multiple :m and import commands here, it would be simpler if they were all dealt with together in playCtxtCmd. I realise this means changing the types a bit more radically, though.

  Changed 20 months ago by simonmar

Oh, one more thing: we should have test cases for the new features, including the tricky cases like multiple imports and combinations of imports and :m, and re-loading of source files to trigger the replay functionality.

follow-up: ↓ 21   Changed 20 months ago by SamAnklesaria

Thanks for the comments. parseName relies internally on a function from HscMain? that does just that: name parsing. I don't think it can handle import declarations, which is why I wrote hscImport. As I removed the "import" from the string the user entered (saving all the import parts in a remembered_ctx seems redundant), it needs to be prepended or the parser won't understand it. Importing twice with different import lists is handled like in source files. Any ":m" commands will overwrite import declarations for the same module. The problem with representing an import in a CtxCmd is that it has a completely different syntax, so storing it in the same structure seems awkward. A new patch should be up soon.

in reply to: ↑ 20   Changed 20 months ago by simonmar

Replying to SamAnklesaria:

Thanks for the comments. parseName relies internally on a function from HscMain? that does just that: name parsing. I don't think it can handle import declarations, which is why I wrote hscImport.

Yes - I'm just suggesting that instead of getImportDecl you call it parseImportDecl and make it parse a complete import declaration for consistency.

As I removed the "import" from the string the user entered (saving all the import parts in a remembered_ctx seems redundant), it needs to be prepended or the parser won't understand it.

My suggestion is to not remove it in the first place, that seems simpler.

Changed 20 months ago by SamAnklesaria

Changed 20 months ago by SamAnklesaria

  Changed 20 months ago by SamAnklesaria

  • status changed from new to patch

  Changed 19 months ago by simonmar

  • status changed from patch to closed
  • resolution set to fixed

Applied, thanks for the contribution!

Thu Jun 24 20:26:32 PDT 2010  amsay@amsay.net
  * trac #2362 (full import syntax in ghci)
  'import' syntax is seperate from ':module' syntax
Note: See TracTickets for help on using tickets.