reflection: allow names to return using-ed names
#54609
Conversation
src/module.c
Outdated
| // Add the name of `usinged` itself, unless the user requested `all=true` and it's | ||
| // a submodule of `m`, since then its name would have already been added by | ||
| // `all=true`, since it's a binding in `m`. | ||
| if (!all || usinged->parent != m) { |
There was a problem hiding this comment.
I'm not sure this is enough to avoid duplicates. For example the name might also be exported. We could also de-duplicate in the caller though.
There was a problem hiding this comment.
Yeah... I've been thinking about how to eliminate the duplication, and it seems it might be better to start by organizing the interface of names. With the current design and implementation, all/imported and usings are not necessarily orthogonal, so the duplication is inherent in the definitions.
By defining names as names(x::Module; non_public::Bool=false, generated::Bool=false, imported::Bool=false, usings::Bool=false) and refactoring the code, I think we can make the handling of such duplications more clear.
There was a problem hiding this comment.
The refactoring mentioned above involves more changes, so..., for now in this PR, I want to either 1) have names return a Set to eliminate duplicates on the names side (technically this is a breaking change?), or 2) add a warning about the possibility of duplicates in the names docstring. Which do you think is better?
There was a problem hiding this comment.
I've decided to go with the second option. Regarding the initial comment, I've found that this code itself is not necessary, so I will just remove it. The duplication issue remains (such as when there is a name conflicting with using-ed one), but at least there is a warning, so it shouldn't be a big problem.
aviatesk
force-pushed
the
avi/36529
branch
2 times, most recently
from
f01d4e3 to
ca41f1a
Compare
|
Further refactoring might be needed for |
aviatesk
force-pushed
the
avi/36529
branch
2 times, most recently
from
edb44e5 to
9f2e685
Compare
This commit makes it possible for `names` to return `using`-ed names
as well:
```julia
julia> using Base: @assume_effects
julia> Symbol("@assume_effects") in names(@__MODULE__; usings=true)
true
```
Currently, to find all names available in a module `A`, the following
steps are needed:
1. Use `names(A; all=true, imported=true)` to get the names defined by
`A` and the names explicitly `import`ed by `A`.
2. Use `jl_module_usings(A)` to get the list of modules `A` has
`using`-ed and then use `names()` to get the names `export`ed by
those modules.
This method is implemented in e.g. REPL completions, but it has a
problem: it could not get the names explicitly `using`-ed by
`using B: ...` (#36529, #40356, JuliaDebug/Infiltrator.jl#106, etc.).
This commit adds a new keyword argument `usings::Bool=false` to
`names(A; ...)`, which, when `usings=true` is specified, returns all
names introduced by `using` in `A`.
In other words, `usings=true` not only returns explicitly `using`-ed
names but also incorporates step 2 above into the implementation of
`names`.
By using this new option, we can now use
`names(A; all=true, imported=true, usings=true)` to know all names
available in `A`, without implementing the two-fold steps on application
side.
As example application, this new feature will be used to simplify and
enhance the implementation of REPL completions.
- fixes #36529
Co-authored-by: Nathan Daly <[email protected]>
Co-authored-by: Sebastian Pfitzner <[email protected]>
| If `imported` is true, then names explicitly imported from other modules | ||
| are also included. Names are returned in sorted order. | ||
| are also included. | ||
| If `usings` is true, then names explicitly imported via `using` are also included. |
There was a problem hiding this comment.
Thanks for pointing that out. I plan to address this issue as well in #54659.
|
It seems a bit confusing that |
|
Yeah it would be a breaking change. |
This commit makes it possible for `names` to return `using`-ed names as
well:
```julia
julia> using Base: @assume_effects
julia> Symbol("@assume_effects") in names(@__MODULE__; usings=true)
true
```
Currently, to find all names available in a module `A`, the following
steps are needed:
1. Use `names(A; all=true, imported=true)` to get the names defined by
`A` and the names explicitly `import`ed by `A`.
2. Use `jl_module_usings(A)` to get the list of modules `A` has
`using`-ed and then use `names()` to get the names `export`ed by those
modules.
This method is implemented in e.g. REPL completions, but it has a
problem: it could not get the names explicitly `using`-ed by `using B:
...` (JuliaLang#36529, JuliaLang#40356, JuliaDebug/Infiltrator.jl#106, etc.).
This commit adds a new keyword argument `usings::Bool=false` to
`names(A; ...)`, which, when `usings=true` is specified, returns all
names introduced by `using` in `A`.
In other words, `usings=true` not only returns explicitly `using`-ed
names but also incorporates step 2 above into the implementation of
`names`.
By using this new option, we can now use
`names(A; all=true, imported=true, usings=true)` to know all names
available in `A`, without implementing the two-fold steps on application
side.
As example application, this new feature will be used to simplify and
enhance the implementation of REPL completions.
- fixes JuliaLang#36529
Co-authored-by: Nathan Daly <[email protected]>
Co-authored-by: Sebastian Pfitzner <[email protected]>
LilithHafner
added
the
needs compat annotation
This commit makes it possible for
namesto returnusing-ed names as well:Currently, to find all names available in a module
A, the following steps are needed:names(A; all=true, imported=true)to get the names defined byAand the names explicitlyimported byA.jl_module_usings(A)to get the list of modulesAhasusing-ed and then usenames()to get the namesexported by those modules.This method is implemented in e.g. REPL completions, but it has a problem: it could not get the names explicitly
using-ed byusing B: ...(#36529, #40356, JuliaDebug/Infiltrator.jl#106, etc.).This commit adds a new keyword argument
usings::Bool=falsetonames(A; ...), which, whenusings=trueis specified, returns all names introduced byusinginA.In other words,
usings=truenot only returns explicitlyusing-ed names but also incorporates step 2 above into the implementation ofnames.By using this new option, we can now use
names(A; all=true, imported=true, usings=true)to know all names available inA, without implementing the two-fold steps on application side.As example application, this new feature will be used to simplify and enhance the implementation of REPL completions.
usinged names in a module #36529