Open timotheecour opened 3 years ago
So let's see: The compiler compiles two versions of strutils.nim
for the same program. What could go wrong? Every extern: "nsuParseFloat"
declaration means we produce 2x the same C code, the compilation fails. And that's the first issue that I could come up with, without testing the feature at all, just running it in my head.
Next problem: The compiler reports an error like "Expected type Foo but got type Foo". So ... it's only a couple of dozen "bugfixes" away from working. Complexity breeds complexity, you cannot fight complexity with "let's have more features".
I'm aware of this, include has similar issues; the user is in control and can write:
# in strutils2:
func normalize*(s: string): string {.rtl, extern: "strutils2_nsuNormalize".} =
and then it works, i tried.
furthermore, this can be solved by using module name prefix in extern pattern using $m
, so that user doesn't need to change anything:
# in strutils2:
func normalize*(s: string): string {.rtl, extern: "$m_nsuNormalize".} =
and a single PR can change all the extern declarations in stdlib accordingly with $m_
instead of hardcoding it via "nsuNormalize"
Araq wrote:
Complexity breeds complexity
Before all this std/
and pkg/
shenanigans, if I wanted to override a stdlib module then I could just do it with --path
. Now I need to use patchFile
or maybe this new gizmo.
Cannot the simple "override a module by finding it first in path" mechanism still be made to work or is that now impossible to make work again? Maybe a --path-force
or --path-std
?
Before all this std/ and pkg/ shenanigans, if I wanted to override a stdlib module then I could just do it with --path. Now I need to use patchFile or maybe this new gizmo.
Cannot the simple "override a module by finding it first in path" mechanism still be made to work or is that now impossible to make work again? Maybe a --path-force or --path-std?
I don't see how this comment relates to this PR. --path
and patchFile
still work the same before or after this PR and you can continue to use that; patchFile
is not the same as --path
but anyways pre-exists to this PR.
Cannot the simple "override a module by finding it first in path" mechanism still be made to work or is that now impossible to make work again?
I don't understand this comment, --path
still works. This PR generalizes patchFile
and makes it work on cmdline (or config) instead of just nims file, and allows overriding a module for a select package/module only, optionally, which is not something you can do with either --path or patchFile.
I don't understand your not understanding. Maybe @Araq does.
This PR https://github.com/nim-lang/Nim/pull/18496 improves and generalizes (and in fact subsumes)
patchFile
as follows:--moduleoverride
can be used in cmdline (and therefore, in user config too)the most important aspect of
--moduleoverride
though is that it allows applying the module override in a non-global way, using module prefixes, eg:prefix is an optional comma delimited set of prefixes, eg:
std/strutils,pkg/mypkg,pkg/cligen/bar
it can also contain absolute paths (but not relative paths; a std/ or system/ or pkg/ prefix with a canonical name must be used if the path is not absolute)
example use case 1: breaking change mitigation
this can be used to override a module for just a package, without affecting the behavior of other packages. For eg, when upgrading nim to a new version, if some package foo requires a pre-existing behavior in some stdlib module (eg std/sequtils), you can override std/sequtils as needed and make the override apply only within package foo, unlike a flag like
-d:nimLegacyFoo
which has global effect:example use case 2: custom behavior for a dependency
this is useful for experimentation or if you want to change behavior of a 3rd party package foo in a way that doesn't affect other dependencies that also depend on foo.
example use case 3: replacement patchFile that can work on cmdline/cfg/nims files
(unlike patchFile which is only nimscript)
note
the overridden module can co-exist with the non-overridden module within a project, and this ends up being 2 separate modules (PSym).
use
--processing:filenames
to see what modules are being imported (shows import stacks since https://github.com/nim-lang/Nim/pull/18372)note: rationale for canonical modules names
the way
patchFile
refers to module names has inherent ambiguities, eg:foo/bar/baz
vsfoo/baz
are confused, both as foo_bazfoo_bar/baz
vsfoo/bar_baz
are confused, both as foo_bar_bazfurthermore, it doesn't correspond to how you'd import a module.
future work
--symoverride:mod1.sym:sym2:prefix
: variant which allows to override a fully qualified symbolmod1.sym
bymod1.sym2
within the context of modules matchingprefix
(egsystem.delete
=>system.deleteV0
to get back old behavior within the context of some pkg/foo only).--moduleoverride
is more general (because you can override a whole module as you need) but--symoverride
can be easier to use in some cases especially in cases where a behavior for an API was changed and you want the old behavior for a package without affecting other packages.