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

import at local scope #410

Closed
timotheecour opened this issue Aug 23, 2021 · 16 comments
Closed

import at local scope #410

timotheecour opened this issue Aug 23, 2021 · 16 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Aug 23, 2021

proposal

make import work at local scope, which removes a long standing restriction.

local imports is a basic feature found in lots of other languages with a module system and improves modularity and symbol scoping and can trim down dependencies to "import what you use" in generic code.

examples of languages that support import at local scope

  • python
  • D
  • rust
  • scala
  • ruby
  • typescript
  • js (eg via dynamic import)
  • java (you can use fully qualified name inside a function without needing top-level import)
  • C# (supports using inside a namespace for scoping)
  • matlab, etc

benefits

This allows client/library code to avoid top-level scope pollution when a symbol is only needed in a certain local scope (block or proc). Or, if an operator should only be used in some scope, you can use a local import (refs kaushalmodi/ptr_math#2 for pointer arithmetic operators)

For imports inside generics, this furthermore allows the import depedency to only occur when the generic is instantiated, leading to smaller and more localized dependencies ("import what you use").

Finally, this can also help with circular dependencies, e.g. when an import is inside a generic.

PR

this has been implemented, see nim-lang/Nim#18734

@Araq
Copy link
Member

Araq commented Aug 24, 2021

D changed its scoping rules and introduced special cases for local imports because the results of local imports proved to be unnatural.

Other downsides:

  • it makes it much harder for external tooling to do dependency tracking (most tools don't understand generic instantiations).
  • it encourages more coupling between modules ("it can help with circular dependencies")
  • the benefits of reducing "pollutions" are unconvincing as long as Nim does not attach procs to types so that you can lose useful procs you really need to have in your scope.

@timotheecour
Copy link
Member Author

timotheecour commented Aug 24, 2021

it makes it much harder for external tooling to do dependency tracking (most tools don't understand generic instantiations).

any such tooling worth its salt must also understand constructs such as:

when weirdTarget: # in os.nim
  discard
elif defined(windows):
  import winlean, times

template bar =
  import foo2

template baz(a) =
  # some logic here
  import a
baz(foo3)

and handling local imports for such tool is not anymore challenging than those; in either case, relying on compiler is what should be done to extract import dependencies:

this works:

(nim check --processing:filenames main 2>&1) | grep 'import: .*Processing'

but it'd be easy to add a json output (similar to what's done in D via dmd -deps=<filename>) that would contain compiler outputs such as import dependencies:
nim check --dump:out.json main
out.json would contain a field with all the imports found during compilation, see #412

Such tool shouldn't have to understand generics anymore than conditional compilation, it should rely on either compiler or compilerapi.

D changed its scoping rules and introduced special cases for local imports because the results of local imports proved to be unnatural.

local imports are one of D's great features and both global and local imports are extensively used in D's stdlib; I've used D extensively for many years and used this feature a lot, and am not aware of anyone actually using D complaining about local imports; the style guide https://dlang.org/dstyle.html even recommends:

Local, selective imports should be preferred over global imports

it works just fine.

it encourages more coupling between modules ("it can help with circular dependencies")

quite the opposite, it disentangles dependencies so you only import what you need. What is entangled is the compiler code, which abounds with top-level imports and resorts to workarounds such as extensive use of include files (with all their drawbacks), forward declarations, cast via an interface, fully qualified names which prevents MCS/UFCS etc.

the benefits of reducing "pollutions" are unconvincing.

I disagree, when you have a ton of imports at top-level, figuring out where a symbol comes from is really hard and putting everything in scope is not a good design:

# sem.nim
import
  ast, strutils, options, astalgo, trees,
  wordrecg, ropes, msgs, idents, renderer, types, platform, math,
  magicsys, nversion, nimsets, semfold, modulepaths, importer,
  procfind, lookups, pragmas, passes, semdata, semtypinst, sigmatch,
  intsets, transf, vmdef, vm, aliases, cgmeth, lambdalifting,
  evaltempl, patterns, parampatterns, sempass2, linter, semmacrosanity,
  lowerings, plugins/active, lineinfos, strtabs, int128,
  isolation_check, typeallowed, modulegraphs, enumtostr, concepts, astmsgs

oftentimes such module is only needed inside 1 function or generic, which should have its depedencies self-contained without affecting surrounding code.
This is in particular true with operators that clash (eg json.%, ropes.%, strtabs.%, strutils.% etc).

Using local imports would avoid many of those issues and allow client code to organize their code as they wish instead of being dictated by compiler limitations.

as long as Nim does not attach procs to types so that you can lose useful procs you really need to have in your scope

Attaching procs to type is an orthogonal topic that has plenty of issues but this should be discussed in #380 rather than here; this RFC would be beneficial even if attaching procs were implemented.

@Araq
Copy link
Member

Araq commented Aug 24, 2021

What is entangled is the compiler code, which abounds with top-level imports and resorts to workarounds such as extensive use of include files (with all their drawbacks), forward declarations, cast via an interface, fully qualified names which prevents MCS/UFCS etc.

That code would not be better with local imports -- on the contrary, it would just provide an illusion of cleanliness where there is none. Not to mention that the compiler's entangled dependencies have the root cause in the philosophy of "every feature must always be supported" so that the VM must be able to access the symbol table and if it were for the likes of you even the backend would simply get to use every feature of the frontend willy-nilly. ("The backend can now also do template expansions via the --expandTemplatesLazy:foobar switch. This is useful...")

@Araq
Copy link
Member

Araq commented Aug 24, 2021

@timotheecour
Copy link
Member Author

timotheecour commented Aug 24, 2021

I can restrict the feature to the more conservative from imports, eg: from a import a1, a2, ... and from a import nil in the meantime, this would completely address those concerns and would already provide important benefits (in fact that's what i was doing in an earlier version of that PR).

proc main(group: string) =
  from t12733b import group
  echo group # uses t12733b.group; nothing surprising since it's explicit in the import
  echo t12733b.group # ditto
main("def")

The remaining forms of imports can be discussed separately (import a; import a except b) and are solvable using similar approach as in D, in subsequent work.

@timotheecour
Copy link
Member Author

timotheecour commented Aug 25, 2021

@Araq PTAL: last commit in nim-lang/Nim#18734 (more conservative fix: only support from import for now) now implements the proposed more conservative proposal, which only allows from import at local scope; future work can add other forms of import at local scope after addressing the local symbol hiding issue, possibly in a similar way as done in D.

@Araq
Copy link
Member

Araq commented Aug 25, 2021

  1. It's not clear to me to what extend this feature impacts IC.
  2. I don't like the readability implications, I like to see all the dependencies by looking at the import section at the global scope.
  3. It implies more code updates, soon it will be a new "best practice" that the stdlib and everybody else should use...

And it gets worse: Bad code like:

proc baz(...) =
  from strutils import split
...

proc foo(...) =
  from strutils import split

would not go unnoticed.

For non-generic procs the only reason why the things it imports would not be in the body (assuming we follow this "best practice") would be references to types from a different module in the proc signature. In other words, this feature and the new "best practice" actually encourages bad code (DRY violations) and being even more sloppy with dependencies.

At the same time, we need recursive module dependencies instead of hacks that mitigate the need for it. And we need recursive module deps not because they allow good code practices (they don't), but because they come up all the time when wrapping large C++ code bases.

@Varriount
Copy link

Varriount commented Aug 25, 2021

I don't have an opinion either way, as long as this doesn't have unforeseen consequences on other language mechanisms. I will however note that local imports are not used much in Python. When they are used, it is for a couple of reasons:

  • The module being imported may not be available when the program is first started.
  • Importing the module has a large performance cost, and the imported functionality is only conditionally used.
  • Plugins.

With regards to recursive dependencies, ideally the code reordering mechanism would be improved to the point where it's "production quality" (if that is possible). Right now it technically works, but causes various bugs, and tends to reorder compiler errors as well.

@timotheecour
Copy link
Member Author

timotheecour commented Aug 26, 2021

I don't like the readability implications, I like to see all the dependencies by looking at the import section at the global scope.

it's objectively worse, from first principles:

  • self-contained declarations are preferable: with top-level imports, you don't know why a given module is imported or where it's used; when refactoring you need to move the declaration and remember to remove the corresponding import if exclusive to this declaration
  • putting all declarations at top-level increases the number of symbols in scope, increasing chances of conflicts and resulting workarounds (fully qualified names, affecting MCS, or things like foo.bar[:T])

the dependencies can be obtained by grepping for import's just as easily and no less robustly that if they were at top-level, or, more robustly, via --processing:filenames or --dump:out.json to get dependencies, or by looking at generated docs which contain all imports.

this feature and the new "best practice" actually encourages bad code (DRY violations) and being even more sloppy with dependencies

the opposite is true. The proposed feature reduces the number of dependencies that end up being processed; not compiling code will always be faster than any other approach;

this won't import std/times, pkg/regex unless fn is instantiated:

proc fn[T](a: T) =
  from std/times import cpuTime
  from pkg/regex import re
  ...

It actually encourages (but doesn't require) being less sloppy about dependencies, by localizing where a dependency is actually needed, so that it's clear who needs what and helps refactoring/re-designing; once you know an API is the only one that uses a heavy dependency foo that you want to get rid of, it's easier to reason about the code.

At the same time, we need recursive module dependencies instead of hacks that mitigate the need for it.

that's an orthogonal topic (see #416), and the feature is still as usesful in presence of cyclic imports (as evidenced by the case of D which makes extensive use of both cyclic depdencies and local imports, and recommends local imports in their style guide).

I'm explicitly not making any style guide recommendation in this PR, merely allowing a pattern that is both useful and leads to better code and slimmer depdendencies.

@Araq
Copy link
Member

Araq commented Aug 26, 2021

by looking at generated docs which contain all imports.

No, because it won't pick up imports by unused and exported generic procs that use the feature.

it's objectively worse, from first principles:

Meh, that's just the old boring argument from assuming that you lack editor integration. In real systems, you cursor over the symbol to see where it comes from and the real alternatives would be to write foobar{.from:filename:line:col} instead. Which was never attempted in any production setting ever for obvious reasons.

From these principles we would also always write var x: T = y instead of var x = y.

as evidenced by the case of D which makes extensive use of both cyclic depdencies and local imports, and recommends local imports in their style guide

Then I might as well claim that we don't need the feature because Golang lacks it...

@timotheecour
Copy link
Member Author

timotheecour commented Aug 26, 2021

No, because it won't pick up imports by unused and exported generic procs that use the feature.

nor does nim doc report things like winlean in import section in https://nim-lang.github.io/Nim/os.html because of conditional compilation ...

Unlike conditional compilation, however, it's actually easy to make docgen report the local imports used in declarations (in particular for generics, we visit those during generic pre-pass), so that the docs will still be able to mention local imports (either in global import section or attaching an import section to a declaration in docs, TBD);

so this argument disappears after a small patch to docgen.

Meh, that's just the old boring argument from assuming that you lack editor integration.

I don't know an editor integration that will tell you where an imported module is used. Furthermore, you shouldn't have to use an IDE and compile code just to understand the dependencies in a module; IDE's don't help when reading/browsing code you're not actively developing, which is a very common scenario.

From these principles we would also always write var x: T = y instead of var x = y.

no this doesn't follow form first principles/best practices; this violates DRY; and it definitely doesn't follow from the 1st principles I mentioned above:

  • "self-contained declarations are preferable": you can move around/refactor var x = y without impacting surrounding code; whereas moving around a declaration that imports an exclusive import requires also a separate change that moves the import
  • "putting all declarations at top-level increases the number of symbols in scope": also unrelated

Then I might as well claim that we don't need the feature because Golang lacks it...

you should also mention the fact that go lacks generics which would be the thing that would benefit the most from local imports, due to cutting down on unused dependencies. But more importantly, you're cherry-picking a language that has widely opposite design goals as nim, putting all the emphasis on language design simplicity at the expense of features and user experience.

All languages nim takes inspiration from according to its webpage support some form of local imports, either in any scope (python) or in some more restricted sense (ada, modula); more relevant and to the point, these more modern languages support local imports:

python
D
rust
scala
ruby
typescript
js (eg via dynamic import)
java (you can use fully qualified name inside a function without needing top-level import)
C# (supports using inside a namespace for scoping)
matlab, etc

This feature removes a long-standing limitation and provides significant benefits to manage dependencies and scoping, in particular, cutting down on optional dependencies in generics, which is hard to argue about; I'm not recommending a style guide change that'd require changing a bunch of code and made explicitly no mention of any such guidelines, but it gives users the choice if they wish to benefit from it.

@Araq
Copy link
Member

Araq commented Aug 29, 2021

Well one third of your original proposal is "examples of languages that support import at local scope", this is terrible for lots of reasons and makes arguing for the feature much harder than it would otherwise be:

There many features and many languages and most features are supported by a selected set of languages. So should Nim be the superset of all language features? Hopefully not, so a feature needs to stand on its own merits ("from first principles"). Then you argue that it is:

a) syntax sugar so no harm done.

AND

b) it's a new idiom for generic code enabling "optional dependencies".

I tried to point out that the AND here is a contradiction. So this leaves us with a new feature, enabling more stuff with unforeseen consequences. I heavily oppose the notion of "optional dependencies" and I'm quite sure that they impact IC. ("Recompile this module when its dependencies change!" - "what about its optional dependencies?" - "er...")

But more importantly, you're cherry-picking a language

I didn't: C# does not have the feature, nor do Delphi, Modula 2 and 3, Ada, C++, Fortran. Actually all the languages that Nim took most inspiration from... But as I said, it doesn't matter. Features should stand on its own merits.

@bpr
Copy link

bpr commented Sep 24, 2021

I agree of course that the provenance of the features shouldn't matter, but IMO Ada is an example of a language supporting this. Ada is different than Nim, and an import name in Nim is a with name; use name; in Ada. The "with" is the thing that has to occur at the top level for any dependency (redundant IMO) and the "use" is what allows you to refer to the item without the package name dot prefix. Use can be localized, and that is the suggested style in Ada, as per

https://www.adaic.org/resources/add_content/docs/95style/html/sec_5/5-7-1.html

So I think Ada is in that list of those languages that support the feature, modulo the redundant "with" clause.

I like the feature and think Nim would benefit from it. I have yet to see a rational reconstruction of Nim from "first principles" so I can't say local imports follow from or are rejected by these principles. Generally, I like lexical scope, and the possibility of nesting, even if in practice two levels of nesting is almost always enough.

@Araq
Copy link
Member

Araq commented Sep 24, 2021

To the best of my knowledge with name inside a procedure is not supported in Ada but even if it is, I've never seen it in production code and it's not used to introduce new exciting optional dependencies for generic code...

@hugosenari
Copy link

I can restrict the feature to the more conservative from imports, eg: from a import a1, a2, ... and from a import nil in the meantime, this would completely address those concerns and would already provide important benefits (in fact that's what i was doing in an earlier version of that PR).

With this restriction, problem detailed at #380 may appear more since import objs couldn't be used , we should be using from obj import Obj, hash, == but someone could use from objs import Obj instead.

I'm not arguing against the feature, any other option for 'local imports hiding local symbols'?

@Araq
Copy link
Member

Araq commented Oct 14, 2022

Months passed, nothing changed my opinion. It looks un-idiomatic and dangerous. In the best case it would "only" produce a massive amount of work and implementation bugs, draining resources from IC and cyclic modules which are much more useful.

@Araq Araq closed this as completed Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants