Skip to content
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

Local modules #283

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

Conversation

goldfirere
Copy link
Contributor

@goldfirere goldfirere commented Oct 13, 2019

This is an alternative to #205 that appears to be more compositional.

Rendered

@deepfire
Copy link

I consider #205 obsoleted by this proposal.

My only hope is that we have a volunteering implementor sooner, rather than later!

@deepfire
Copy link

The only part from #205 that is missing is details of module namespace merging & reexports, which could potentially be migrated over, I think.

I mean, surely we want to be able to allow the qualified names to be transparently re-exported, right?

2. Introduce a new declaration form (allowed only at the top level of a
module) to declare new modules called *local modules*. Here is the BNF::

decl ::= ... | [ 'import' ] 'module' modname [ export_spec ] 'where' decls
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the [ 'import' ] correct? I imagine import and where do not go together. There are a few other suspicious [ 'import' ]s below as well.

@treeowl
Copy link
Contributor

treeowl commented Oct 13, 2019

I've just started to read the proposal, but ran into this:

This is limiting if a user wants to, say, change a class into a type family that returns a constraint; downstream users have to change their import statements for what was meant to be a local refactoring.

That sort of change simply isn't local, particularly because a class and a type family are genuinely different. I imagine you can repair this bullet point.

@chshersh
Copy link

@goldfirere Thanks for writing this proposal and working in this direction! I appreciate a lot the work on UX improvements to imports and reexports. However, when reading this proposal, I couldn't get rid of the feeling that this is two proposals in one:

  • Proposal 1: qualified reexports
  • Proposal 2: local modules

I personally care a lot about the following feature from your proposal and from the previous proposal about Structured imports.

module MyPrelude ( qualified module BL
                 , qualified module BS
                 , Set
                 , qualified module Set ) where

import qualified Data.ByteString.Lazy as BL
import qualified Data.ByteString as BS
import Data.Set ( Set )
import qualified Data.Set as Set

As a maintainer of one of the popular alternative preludes, I find this feature extremely valuable. This can be a 2x improvement to my daily programming in Haskell. I could even say it's a 10x improvements to keep up with modern trends. I just can't put into words how important this minor syntactic feature and how much more convenient it makes to develop programs in Haskell.

On the other hand, I have some troubles understanding the motivation and usefulness behind local modules and what exact problem do they solve. And the proposal is mostly about this local modules feature (hence the name). However, this feature looks complicated with a lot of specification tasks. And according to my experience, there probably going to be a long discussion on improving specs for this feature to cover various corner cases, bikeshedding the syntax, discussing a motivation behind such a compiler complication, etc. All this can stop such a useful feature as qualified reexports from being merged which is a shame.

Would it make sense to split this proposal into two?

  1. QualifiedReexports
  2. LocalModules

I don't see how the part of LocalModules is important for QualifiedReexports. I can only see how QualifiedReexports are necessary for LocalModules to work properly. So one extension can be built on top of another.

@gbaz
Copy link

gbaz commented Oct 14, 2019

I've wanted LocalModules for a long time! I think maybe Yitzchak Gale had a proposal for such a thing back in the day too?

One of my use cases then was autogenerated datatypes to correspond to tables of a database -- each a record, with fields named for each of the columns. Two tables would want to share column names. But to do so, each would have to be declared in its own module, each designed to be imported qualified. So a single schema would potentially give rise to many files, one per table. With a proposal like this, each table can get its own local module within the same actual file -- much nicer!

The ability to define "hidden" helper datatypes is great too for modules designed to be imported unqualified. I've often written modules where I want to export "all but three" things or the like, and its irritating to have to write a whole export list instead of just keeping those three helper things entirely local. I think that in fact many modules I've worked on have this characteristic, and in all those cases, this will really cut down on verbosity.

@deepfire
Copy link

deepfire commented Oct 14, 2019

Two tables would want to share column names. But to do so, each would have to be declared in its own module

@gbaz, strictly speaking, this is not necessarily true, since #160 was accepted -- although I wholeheartedly agree with the rest of your comment!

Copy link
Contributor

@nomeata nomeata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I miss some discussion about the interaction with Backback. At least backpack also has a multiple-module-per-file feature, right?

Comment on lines 221 to 225
7. Every ``class``, ``data``, ``newtype``, ``data instance``, and ``newtype
instance`` declaration implicitly creates a new local module. The name of
the local module matches the name of the declared type. All entities (e.g.,
method names, constructors, record selectors) brought into scope within the
declaration, including the type itself, are put into this local module.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a nice convenience feature on top of the existing proposal, but is not crucial to it, right, as you can emulate that behavior with a few extra lines of wrapping the type in a module.

(Not that I dislike it, just asking for better understanding of the design space.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this seems orthogonal to the main proposal, and perhaps best included as an additional extension on top of it.

Comment on lines 272 to 273
If the ``import`` keyword is included, then all entities brought into
scope qualified are also brought into scope unqualified.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this unqualify one level of module nesting depth? If Foo.hs exports module M1 which exports M2 which exports bar, and I write

import Foo ( import module M1 )

is M2.bar or bar in scope?


* Other than corner cases around ambiguity, this proposal is backward compatible; it is not "fork-like".

* Proposal `#160`_ allows users to suppress field selectors, thus ameliorating a small part
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Proposal `#160`_ allows users to suppress field selectors, thus ameliorating a small part
* Proposal `#160`_ allows users to suppress field selectors, thus `ameliorating <https://www.merriam-webster.com/dictionary/ameliorate>` a small part

Copy link
Contributor

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am enthusiastically in favour of this proposal. I have a half-written draft of almost exactly the same proposal sitting on my hard drive...

As an anecdotal point in its favour, I implemented a very similar scheme for another programming language and it was wildly successful. The simple approach of "modules have names like everything else, and you can import them and export them as normal" turns out to be very flexible.

I also found that it formalizes quite nicely as a set of deduction rules for constructing the relations in question. (The existing formalization paper also reads to me like an algorithmic version of some deduction rules.) Such a formalization might be a nice contribution for this proposal, and would make it nice and clear what the delta is.

Comment on lines 221 to 225
7. Every ``class``, ``data``, ``newtype``, ``data instance``, and ``newtype
instance`` declaration implicitly creates a new local module. The name of
the local module matches the name of the declared type. All entities (e.g.,
method names, constructors, record selectors) brought into scope within the
declaration, including the type itself, are put into this local module.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this seems orthogonal to the main proposal, and perhaps best included as an additional extension on top of it.

of the enclosing ``instance`` declaration: the ``data``\/\ ``newtype``
module is *not* nested within the class module.

8. Local modules may be extended via the declaration of another local module
Copy link
Contributor

@michaelpj michaelpj Oct 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we need this, but I have a guess: it's to preserve the existing behaviour when modules are from different sources are renamed to the same name (e.g. import F as N; import G as N). If that is the main reason for including this, then I think we should instead consider simply banning this implicit merging if LocalModules is enabled, or have it give an ambiguous name error.

Haskell currently has three related restrictions:

* *Modules*, a set of declarations that share a namespace and perhaps are
surrounded by an abstraction barrier, coincide with *source files*, the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would help to have a term for "modules which are a source file". Perhaps "file module"?

I would also mildly prefer it if we chose our terminology such that we could use "module" unqualified to refer to the namespacing concept, and some other term for the rather special modules which you get when declaring them at the top-level of a source file.


Note that the declaration form includes the word ``module`` to distinguish
it from a normal ``import`` which induces a dependency on another file. An
``import module`` declaration cannot induce a dependency.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big 👍

In an ideal world I would love it if we had two keywords, something like open (brings names from a module into scope, corresponding to import module) and link(incurs a dependency on a file module and brings the fully qualified module name into scope), with our current import being a combination of the two.

5. A local module declaration brings into scope names listed in its export
list. These names are always brought into scope qualified by the local
module name, unless that module name is ``_``. If the declaration includes
the ``import`` keyword, the names are also brought into scope unqualified.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the optional import on a module declaration? All it does is save one line to go import <modname>.

y = T.x

There will be two identifiers ``T.x`` in scope: both the one imported from ``T`` and the record selector
in the type ``T``. This situation will lead to an error, as do other sources of ambiguity.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

I think there is a good design principle here, something like:

Allow ambiguous names to be declared but not referenced, and rely on module renaming to let the user resolve them if necessary.

There will be two identifiers ``T.x`` in scope: both the one imported from ``T`` and the record selector
in the type ``T``. This situation will lead to an error, as do other sources of ambiguity.

* The ability to detect dependencies of a module by parsing only a prefix of the module is retained.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯


However, it would be nice to separate the treatment of compilation units and source files,
as well. This would allow, for example, the inliner and specializer to make decisions with
respect to more definitions (if the compilation unit is larger than the source file).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also allow a much simpler implementation of mutually recursive modules: the modules in a SCC form a compilation unit and are processed together, using the usual fixpoint computation.


2. Haskell currently requires three distinct concepts to coincide: *compilation units* are the
chunks that go through the compiler all at once, *source files* are distinct files on disk,
and *modules* are groups of related definitions and can define an abstraction barrier.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯


I see a few future directions along these lines, but I leave it to others to flesh these out.

1. We can imagine *parameterized local modules*, where all the functions defined therein share
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In particular, I think this proposal is a common subset of almost any serious parameterized module proposal, so is a pretty reasonable thing to implement.

@goldfirere
Copy link
Contributor Author

@deepfire

I consider #205 obsoleted by this proposal.

I hope this isn't out of discouragement! Many of the ideas here grew due to my (quiet) following of that proposal.

The only part from #205 that is missing is details of module namespace merging & reexports, which could potentially be migrated over, I think.

Do you have an example? I'm not sure what this means. Feel free to link to an example in your proposal or discussion instead of creating one fresh.

@Ericson2314

Is the [ 'import' ] correct [in import module M where ...]?

Yes. The import means that all the definitions in the module enter the outer scope unqualified. Without the import, the definitions stay qualified by their module.

@treeowl

[Changing a class to a type family isn't a local change and should affect downstream users.]

I disagree. Imagine we have

class C a where
  meth :: a -> a

I might later prefer

type family C a :: Constraint where
  C Int = ...
  C Bool = ...
  C a = ...

meth :: C a => a -> a
meth = ...

From users' standpoint, these are used equivalently (if users don't write instances -- maybe that's the part that should be clarified).

@chshersh

this is two proposals in one

Very good observation. Somehow, this all formed in my head as one idea, but I agree with you that it's really two proposals. But they do interact. For example, if we just had qualified-exports, you couldn't export locally defined identifiers qualified. You would need a shim module just to rejigger the exports. And if you want multiple different qualifications (imagine a module exporting a datatype, some common operations, and then both Lazy and Strict operation sets), now you need many modules. Seems easier and more coherent to do this all together.

@nomeata

I miss some discussion about the interaction with Backback. At least backpack also has a multiple-module-per-file feature, right?

I don't think so. Backpack seems all about signatures, which this proposal does not address. Please correct me if I'm wrong, but I don't see a meaningful interaction here.

@michaelpj

The existing formalization paper

Thanks for the link! I was not familiar with that paper. I cannot say I'm going to run off and formalize this proposal in the style of that paper right now, but I agree that such a thing would be nice. I would welcome and review closely a contribution in this form from another member of the community.

I'm not sure why we need [module extension]

It was to allow mixing other definitions with e.g. constructors of a type declaration, so that the other definitions could be in the implicit module created along with the type. I've added some text about this. This feature is very inessential.


I believe I have responded to other points in an update I am about to push.

Thanks for all the commentary!

@deepfire-pusher
Copy link

deepfire-pusher commented Oct 15, 2019

The only part from #205 that is missing is details of module namespace merging & reexports, which could potentially be migrated over, I think.

Do you have an example? I'm not sure what this means. Feel free to link to an example in your proposal or discussion instead of creating one fresh.

As a starting example, let's discuss entries #6 and #8 in the exports table: https://github.com/deepfire/ghc-proposals/blob/master/proposals/0000-structured-imports.rst#2124comparative-case-analysis

The idea is that if we want first-class support for qualified names, we should be able to re-export them without explicitly mentioning -- just as for the regular names.

@goldfirere
Copy link
Contributor Author

Line 6:

module M ( qualified O ) where

import N
import P

You suggest that this should export all the identifiers qualified with O. as imported from N and P.

I believe that same behavior is included in my proposal, as part of specifications 8 and 11. (8) says that local modules may be extended. O must be a local module imported from N. Then when we import a local module of the same name from P, the local module O in M is extended. It is then re-exported. (My syntax requires the module keyword in the export list, but that's the only change.)


Line 8:

module M ( module N ) where

import N
import qualified P as N

You suggest this should export all of N's and P's exports unqualified.

I actually disagree with this behavior, as it clashes with the existing semantics for exporting a module, which says that this exports all identifiers in scope both unqualified and qualified with the particular module name. If we remove the qualified from your import, then you should get the behavior you want.

One thing my proposal does not provide is the ability to export a qualified module as unqualified. Individual identifiers may, of course, be listed in an export list, but I offer no way to unqualify an entire module in an export list. Perhaps we could add import module M as an export item, but I think that's a step too far.


Have I accurately captured your examples? Thanks for including them!

@michaelpj
Copy link
Contributor

Thanks for the link! I was not familiar with that paper.

I do think it would be good to consider it in the related work at least.

I cannot say I'm going to run off and formalize this proposal in the style of that paper right now, but I agree that such a thing would be nice. I would welcome and review closely a contribution in this form from another member of the community.

Very fair. I will try and find some time to write something down.

@deepfire-pusher
Copy link

deepfire-pusher commented Oct 16, 2019

Line 6:

module M ( qualified O ) where

import N
import P

You suggest that this should export all the identifiers qualified with O. as imported from N and P.

I believe that same behavior is included in my proposal, as part of specifications 8 and 11. (8) says that local modules may be extended.

Citing (8) from "Structured imports/exports":

Local modules may be extended via the declaration or importing of another local module of the same name.

So O would be the local module exported by N -- sounds great, that point is covered then!

Line 8:

module M ( module N ) where

import N
import qualified P as N

You suggest this should export all of N's and P's exports unqualified.

I actually disagree with this behavior, as it clashes with the existing semantics for exporting a module, which says that this exports all identifiers in scope both unqualified and qualified with the particular module name. If we remove the qualified from your import, then you should get the behavior you want.

That's a fair point, I agree that the #205 made a mistake of including the unqualified names when they weren't imported from their origin module.

If we remove the qualified from your import, then you should get the behavior you want.

Sounds great, that point is covered as well, then.

This raises one question though -- would we still export P's qualified names (i.e. local modules)?

@mpickering
Copy link
Contributor

I found the structure of the proposal quite hard to understand because it starts from some perceived problems rather than describing the solution with an example. I find section 1 too high-level and section 2 too low-level.

In the motivation section, point 2 seems quite unrelated to point 1. There seems to be two different things going on,

  1. The ability to explicitly define local modules
  2. Also some new behavior where data types and classes define new namespaces

These should be separate proposals in my opinion and I am sympathetic to both ideas. However, I wonder how this correct treatment of records interacts with the many other proposals to do with field selectors which appear to move in the other direction.

Specific comments

  • "allowed only at the top level of a module" - does this mean that you can nest modules as deep as you like or not?

  • Indenting a very large portion of a file in order to indicate scope seems quite annoying.

  • The proposal also pays no attention to how this could be implemented. The coincidence between modules and files is probably quite difficult to disentangle in the compiler. Especially as the author doesn't volunteer to implement the proposal then someone should pay some attention about how easy or possible the implementation is before the idea is considered further.

  • The proposal also doesn't mention one of the main benefits of local modules being that you could define and use template haskell expressions in the same file, if you placed them in separate modules. Is that possible with this proposal? If it's not then it seems like a wasted opportunity.

  • I am a bit unsure about the difference between "import" and "import module". It seems that if you want to import anything for use in any module you have to import it at the top level? So all local modules in a file will have to be recompiled if any of the dependencies of any of the modules change?


Overall I am really keen to see this proposal move forwards, I think it would be a large improvement to the language and seems to be quite elegant. In order for things to move forward I think the proposal needs to be clarified significantly to separate the two distinct points.
I could even be persuaded to implement it when the time comes.

@mpickering
Copy link
Contributor

I also didn't read any of the comments so if these questions are just from reading the proposal.

@michaelpj
Copy link
Contributor

The proposal also doesn't mention one of the main benefits of local modules being that you could define and use template haskell expressions in the same file, if you placed them in separate modules. Is that possible with this proposal? If it's not then it seems like a wasted opportunity.

I am a bit unsure about the difference between "import" and "import module". It seems that if you want to import anything for use in any module you have to import it at the top level? So all local modules in a file will have to be recompiled if any of the dependencies of any of the modules change?

The compilation unit is still the file, even thought there may be multiple modules in the file. I think that means that:

  • Since the restriction on use of TH is really about compilation units, not files, I don't think this will help there.
    • Multiple compilation units in a file seems much harder to me - this proposal is only really about how names are resolved, not how the compilation process proceeds.
  • All the local modules in a file are in the same compilation unit (the file), so they will always be (re-)compiled together.

@deepfire
Copy link

My apologies, but in the interest of moving things forward --

Do we have consensus on that this proposal should be split?

If yes, what should be the split?

From what I can see there are several components:

Citing @mpickering:

  1. The ability to explicitly define local modules
  2. Also some new behavior where data types and classes define new namespaces

..and also, the part that concerns me the most -- allowing modules (or, depending on your perspective, qualified names) to travel across the import/export boundary -- the part that #205 was covering.

@goldfirere
Copy link
Contributor Author

Just a quick note to say I'm on holiday this week and will update this next week. Sorry to lose momentum here!

@goldfirere
Copy link
Contributor Author

@michaelpj

See the impending update to the proposal for further incorporation of your remarks.

@deepfire

would we still export P's qualified names (i.e. local modules)?

Yes. I've added some clarification in this regard.

@mpickering

In the motivation section, point 2 seems quite unrelated to point 1. There seems to be two different things going on:

  • The ability to explicitly define local modules
  • Also some new behavior where data types and classes define new namespaces

I agree that these are separable. But they go nicely together. I have added a point under Alternatives that suggests dropping the second point above. I think the proposal is fine without this extra.

I wonder how this correct treatment of records interacts with the many other proposals

I've added some text about this at the end of the Effects section.

"allowed only at the top level of a module" - does this mean that you can nest modules as deep as you like or not?

You can nest as deeply as you like. Note that it says "top level of a module", not "top level" or "top level of a file". The point here is that you cannot declare a local module within a let or a where. I've clarified the text.

Indenting a very large portion of a file in order to indicate scope seems quite annoying.

This is a good point. But Java, C++, and C# (at least) do this all the time. (Yes, I know it's not semantically significant there. The conventional style for these languages do it anyway.) I'm happy to consider alternative syntaxes, but given the success of long indented regions elsewhere, I'm not going to worry unduly about this.

The proposal also pays no attention to how this could be implemented.

Very true -- which is why I don't volunteer to implement. The good news is that the entire proposal deals only with the renamer. Indeed, I imagine if we gave GHC's datatype Name some more structure, an early pass could likely desugar local modules into constructs involving qualified names. Local modules would have to be persisted in interface files, but I don't actually think this would be all that hard to do.

More broadly, the lack of concern of implementation is intentional. This proposal strikes me as a convenient, compositional approach to modules. If it's hard to implement, then perhaps that means that the implementation is not working as well as it can for us. There is also the possibility of partial, future-compatible implementations of this proposal that would be easier to fit with today's GHC.

One choice is made for practicality: the fact that the new import module declaration is easily distinguished from old fashioned imports, meaning that file-dependency analysers still do not have to parse the entire module.

The proposal also doesn't mention one of the main benefits of local modules being that you could define and use template haskell expressions in the same file, if you placed them in separate modules. Is that possible with this proposal? If it's not then it seems like a wasted opportunity.

The proposal doesn't mention this because it's not true. Local modules do not affect compilation order or dependency. This allows local modules to be mutually recursive without issue. I've added an Alternative considering your implied proposed design.

I am a bit unsure about the difference between "import" and "import module". It seems that if you want to import anything for use in any module you have to import it at the top level? So all local modules in a file will have to be recompiled if any of the dependencies of any of the modules change?

This question seems to suggest that you're thinking of a world where "module" = "compilation unit". This proposal moves us away from this coincidence. import finds another file on disk somewhere, loads it, and then brings its identifiers into scope. import module just does name swizzling. Both conform to the definition of the English word "import", but they're really two separate operations. @michaelpj suggests (#283 (comment)) that we break traditional import into two steps (one with file loading and the other with name swizzling), but I personally think that ship has sailed (as does @michaelpj from the looks of it).

In order for things to move forward I think the proposal needs to be clarified significantly to separate the two distinct points.

I think the second point above is really confined to one bullet (point 7) and can easily be removed. Perhaps also the introduction bit (above Motivation) is overlong, and threw you off. Are there other areas of clarification needed?

I could even be persuaded to implement it when the time comes.

Hooray! :)


I don't yet feel motivated to split this proposal. Instead, I have now included a menu of Alternatives; many of these suggest dropping individual points of the specification in this proposal. I personally like the proposal in its current form, but I'm fine with enacting these Alternatives.

/remind me in a week to submit this if there has been silence.

@reminders-prs
Copy link

reminders-prs bot commented Oct 30, 2019

@goldfirere set a reminder for Nov 6th 2019

@Ericson2314
Copy link
Contributor

Ericson2314 commented Oct 30, 2019

@goldfirere I have yet to really dive into this, but I do feel the need for some caution. With the Dependent Haskell work, we have your thesis to lay out the overall trajectory in great detail so I have no problems with individual proposals along that path. As much as I love the overall thrust of the proposal and the future work, I don't really know exactly where we are going and am worried about making mistakes we will regret. I'm not saying we need another thesis but....I would like to have something. If there are ways we can improve the "compilation units = modules = files" assumptions and tech debt in GHC without committing to new interfaces, that is also good.

[For example, I think Haskell's structured module names today could well be viewed as a mistake. It would be much cleaner to say "Foo.Bar always means Bar within Foo, but we cannot do that. Yet structured module names were supposed to conservatively anticipate a better module system! I worry about similarly anticipating some good thing and then it turns out we screwed up.]

I really hate being any less than completely ecstatic about this; I really do think this is the most important way to develop the language along with Dependent Haskell.

@goldfirere
Copy link
Contributor Author

There is no grand plan (that I have) around modules and compilation units. (Maybe that's the problem you're worried about!) But perhaps what you're suggesting is that we should work out the compilation unit stuff before committing to this proposal, to make sure it all works together. I'm fine with that. But unless someone stands up to say they are going to do that (and in a somewhat timely fashion), I don't want to stop this proposal indefinitely for some grand master plan to appear.

@maralorn
Copy link
Contributor

maralorn commented Nov 1, 2019

I think this a proposal is great.

I have a specific question regarding the behavior of the flag. Does it need to be enabled to import qualified modules?

I consider a user having the following import:

import qualified Foo

right now the user can be sure that this will never include something like Foo.Bar.baz. (If I don‘t overlook any features.) With this extension that is going to be possible.

So: The LocalModules flag is certainly needed for exporting qualified modules. But will it also be required for importing qualified modules?

If yes this will tend to force the extension on library users. If no this will change the possible meanings of an import statement in Haskell quite a bit. Someone not familiar with this change might get very confused by a e.g. Text.Encoding.decode in the code when there is no import statement for something called Text.Encoding.

It‘s probably a cost worth paying. But I hadn‘t seen it mentioned before.

Comment on lines +786 to +797
#. Let the ambient QEnv of the local module (call it ``M``, located within
top-level module ``Top`` and under other local modules ``modids``) be ``E``.

If ``exports`` is given, the export QEnv
of ``M`` is the result of interpreting ``exports`` with respect to ``E``.

If ``exports`` is not given, then the export QEnv ``X`` of ``M``
is the subset of ``E`` containing all mappings to original names that
appear within ``M``. That is, ::

X = { qual ↦ orig | qual ↦ orig ∈ E
, orig = (Top, modids.M._, _occ) }
Copy link
Contributor

@nomeata nomeata Oct 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think this implies the following behavior. Intended?

module Top
module qualified M where foo = True
module qualified B where import module qualified M as C
_ = M.B.C.foo 

If I read the spec correctly, this would work: The ambient QEnv of M contains the ambient QEnv of Top which has an entry B.C.foo ↦ (Top, M, foo), so it passes the filter, thus B.C.foo is part of the exports, and thus M.B.C.foo comes into scope.

If this is not intended, then you might have to distinguish between the “inherited ambient QEnv” of a scope (stuff coming from declarations in surrounding scopes) and the “intrinsic QEnv” (stuff coming from declarations, including import, in the current scope). Then the exports of a local module would be filtered based on that. Not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get a parse error at import module qualified. I don't think that syntax is supported in this proposal. Do you disagree?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you are right, I made assumptions about the proposal that are not true. Let me try again…

Ok, import module M only ever strips M from stuff that’s already in scope, but no renaming is possible…

How about this, does this work?

module Top
module qualified M where foo = True
module qualified B(foo) where import module M (foo)
_ = M.B.foo 

If I (now hopefully) read the spec correctly, the ambient QEnv of M contains the ambient QEnv of Top which has an entry B.foo ↦ (Top, M, foo), so it passes the filter, thus B.foo is part of the exports, and thus M.B.C.foo comes into scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think so. The proposal says


        X = { qual ↦ orig | qual ↦ orig ∈ E
                          , orig = (Top, modids.M._, _occ) }

where E is the ambient QEnv in the local module M, and modids (in your example) is empty. Thus, X will be foo ↦ (Top, M, foo). Thus the ambient QEnv outside of any local module will be

{ M.foo ↦ (Top, M, foo), B.foo ↦ (Top, M, foo), Top.M.foo ↦ (Top, M, foo), Top.B.foo ↦ (Top, M, foo) }

and that's it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for making you debug my understanding of the proposal. What is the value of E, the ambient QEnv within M, in this example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have laid a trap! Well done.

(More explicitly: I believe @nomeata is drawing attention to the fact that, according to the definitions above, the ambient QEnv in M will include bindings like Top.M.foo ↦ (Top, M, foo), which would end up in X. Yet I don't want M to export Top.M.foo, which would mean that M.Top.M.foo is in scope.)

The best way I can see fixing this problem is to maintain a stack of ambient QEnvs, where entering a local module adds one to the stack, and name lookup proceeds linearly through the stack. I actually don't think this is such a bad way of thinking of things, but it does add an extra layer of complexity. Do you see an alternative?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think it's unavoidable. Not sure if “ambient” is still a good adjective, though, then, it sounds (to me) as if it's already the union.

You might still additionally have to keep “defined here” and “imported” apart to get the semantics of the default export list right, which should not be affected by imports. Matching on the original name after throwing the two together might not work well.

@goldfirere
Copy link
Contributor Author

With some degree of sadness, I'm switching away from this proposal. I would love to have local modules, and would enthusiastically support anyone who took finishing this proposal on. But I should have put my pencil down here some time ago -- this has just been too time consuming, and there are other battles to fight! (This all started due to a rainy Saturday.)

I see two open issues, neither of which have dead-easy answers:

module Top where

module A (a) where
  x = 5
  a = ... x ...
module A (b) where
  x = 6
  b = ... x ...

This looks like a useful-enough pattern, and yet there are two xs with the same original name. We can reasonably ban such xs when they are exported, but these local ones should be allowed to have the same name. Maybe unexported original names get a unique suffix? I don't know -- more design needed here.

  1. See Local modules #283 (comment) and thereabouts. In order to give a reasonable definition to what to export from a local module without an explicit export specification, we need to have some notion of what is defined in the local module. And to do that, it would seem we need to track a stack of ambient QEnvs, so that the lcoal-module export list is determined only by the top QEnv in the stack. Or maybe there's another approach. More design needed here.

As far as I can tell, these are the outstanding issues. Neither has a quick fix, but neither looks like it requires a full reimagining of the proposal. Would anyone like to step in and help get this over the line? I stand by ready to cheer you on.

@nomeata
Copy link
Contributor

nomeata commented Nov 20, 2021

Fair enough. I think this feature isn't "pressing", and rather better later with a even more refined design.

That said, I am confused why you stick to "original name" as "sequence of module names". Unless I am missing something, the module system is managing which mappings from qualified and unqualified to things are in scope where. So we need to identify things (i.e. we need an identifier that supports equality). But I don't see why this internal identifier ("original name") needs to be tied to local modules. It could simply be the how manyth thing the thign is in its (real) module, together with the root module? Or source position. Or even left completely abstractly in this proposal. I believe that would solve issue 1 (and simplify the formalism a bit).

As for issue 2, for that we need a way of saying "thing was defined within the where of a specific local module". Again, I think we can do without ever appealing to names of local modules (so no problem with multiple local modules of the same name, or anonymous local module). If we use source positions to identify the thing, then we can simply compare that against the source span of the module's where. Or at least, it might help; upon further thought one still needs to be careful when a thing originally defined in the current module is available under names that come from imports or from surrounding scopes. Ok, I guess 2 still stands.

But its fair if you want someone else to take over, so just putting this here for that person's benefit.

@aspiwack
Copy link
Contributor

Let me set this proposal as dormant for now, until someone takes up the proposal for the finishing touches.

If someone wants to take over this proposal, please raise your hand in this thread.

@aspiwack aspiwack added Dormant The proposal is not being actively discussed, but can be revived and removed Pending committee review The committee needs to evaluate the proposal and make a decision labels Nov 22, 2021
TristanCacqueray added a commit to TristanCacqueray/ghc-proposals that referenced this pull request Apr 12, 2022
@santiweight
Copy link

santiweight commented Apr 22, 2022

I know this is dormant, but I didn't see this feedback noted anywhere prior...

It appears that most the examples of problematic imports are of the form:

import Namespace.Type (Type)
import Namespace.Type qualified as Type

I don't like the solution of declaring such a pattern at the module exports level, because it doesn't actually clarify the problem, namely that Haskell has no way to associate functions with the types they work over. Also, note that in order to use a function, I have to look at the declaring module's exports, which I think is not great design.

How about something like (modulo syntax):

module Data.Set (Set (..)) where

data Set with (insert, contains, ...) where
  MkSet :: ... -- Ignoring the implementation details of Set

  insert :: Ord a => Set a -> a -> Set a

-- Module in another file (I'm ignoring the multi-module aspect of this proposal)
module Foo where

import Data.Set

foo :: Set Int -> Set Int
foo bar = Set.insert 1 bar

Such a pattern has worked fantastically in other languages (namely object oriented languages) because it aids in code organisation and discoverability. The biggest thing I am seeing in the comments here is this pattern - and I think something like this solution is (a) minimal (b) incremental (c) a good design.

@jvanbruegge
Copy link

The problem with this is (as is in OOP languages) that all functions need to be declared at the site of the datatype definition. With local modules and the merging of modules that Haskell does, you can extend the functions associated with Set

@santiweight
Copy link

For such extensions, you could simply add a new qualified import no?

import Data.Set
import Data.Set.Extra qualified as Set

foo :: Set Int -> Set Int
foo bar = Set.insert 1 bar

other mySet = Set.extraFunction mySet

@jvanbruegge
Copy link

As a user yes, but you could not export Set with your added function

@dpwiz
Copy link

dpwiz commented May 4, 2022

I wonder how this would interact with ImportQualifiedPost.

@jvanbruegge
Copy link

Not at all. ImportQualifiedPost is just different syntax for import qualified _. It is still the same thing

@dpwiz
Copy link

dpwiz commented May 5, 2022

module A ( module M1, module M2, module qualified M3, module qualified M4, module A ) where

could be

module A ( module M1, module M2, module M3 qualified, module M4 qualified, module A ) where

which would allow sorting without lumping qualifieds somewhere in-between:

module A
  ( module A
  , module M1
  , module M2
  , module qualified M3
  , module qualified M4
  , module X
  , module Y
  ) where

@kalhauge
Copy link

kalhauge commented Dec 16, 2022

Thanks to @goldfirere for all his hard work trying to push this through! I'm very interested in this proposal succeeding, but I wanted to try out a last point in the design space. It's available in #564.

The proposal, among other things, tries to solve the remaining two problems in (#283 (comment)) by disallowing multiple local modules (here named namespaces) with the same name.

@vanceism7
Copy link

vanceism7 commented Sep 9, 2023

Hello everyone!

I just wanted to write to signal my intentions to officially take over this proposal and help get it over the finish line. To quote another member of the community, "I'm sort of the village idiot" amongst you all, but I believe this proposal would be immensely helpful for Haskell and I want to give it my best shot.

I'm not committing to taking this on just yet, I want to first read through the entire rendering of the proposal and some of the discussion to make sure I understand things first. But just so it's known that this is being thought about behind the scenes

Thanks all! I'll chime back in when I've finished wrapping my mind around it all

@vanceism7
Copy link

vanceism7 commented Sep 13, 2023

Regarding point 1 as raised by Richard:

module Top where

module A (a) where
  x = 5
  a = ... x ...
module A (b) where
  x = 6
  b = ... x ...

Shouldn't this just result in an error such as

Multiple declarations of ‘x’ found in `Module A`

I think this in line with what would happen in current Haskell if you tried to declare the same bindings multiple times in a top-level-module. This is also how other languages handle this situation.

E.g:
In Go, declaring the same variable in multiple files belonging to the same "module" results in the error:

`x` redeclared in this block (see details) compiler([DuplicateDecl(https://pkg.go.dev/golang.org/x/tools/internal/typesinternal#DuplicateDecl)
some_file.go(35, 7): `x` redeclared in this block
some_file_2.go(59, 7): other declaration of `x` (this error)

In c#

namespace Foo {
	class Bar {
		int x;	
	}
}

namespace Foo {
	class Bar {
		int y;	
	}
}

The following code yeilds the error

The namespace `Foo` already contains a definition for `Bar`

We can reasonably ban such xs when they are exported, but these local ones should be allowed to have the same name.

I guess what I'm saying is that I disagree with this particular statement. Local modules should be allowed to declare variables without regard for the scope of other modules; but if you're extending a module, you should be bound by the rules of not re-using variables within that modules scope, the same as if you were working directly within a single module declaration

Academically, I'm quite naive, so I assume I may be missing some crucial understanding here, but this seems like the straight forward path to handling this situation as far as I can tell

@aspiwack
Copy link
Contributor

@vanceism7 thanks for help. I'm willing to advise on this proposal. Get in touch if you have questions.

@vanceism7
Copy link

Hey everyone, I wanted to chime back in with an update. I spent a good amount of time reading through this proposal, thinking about it, thinking up alternatives, and just generally chewing on the whole thing. @aspiwack and I also had a bit of discourse on this proposal over email, but unfortunately, my life circumstances have changed and I find myself no longer having time to put efforts towards this for the time being. I wish I could've helped at-least move this proposal closer to passing the finish line, but I'm hugely greatful to @aspiwack for spending so much time discussing this with me. If anyone else decides to take this on or is curious, I'm happy to summarize some of the key points of what we talked about. Thanks again everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dormant The proposal is not being actively discussed, but can be revived
Development

Successfully merging this pull request may close these issues.

None yet