Skip to content

Conversation

c42f
Copy link
Member

@c42f c42f commented Jul 2, 2019

CoreLogging is an implementation detail, but the fact that Logging
imports things from there breaks tab completion of Logging.<tab> even though these
symbols are exported again. Fix this by explicitly creating bindings for
each imported symbol.

CoreLogging is an implementation detail, but the fact that Logging
imports things from there breaks tab completion for the parts of the
public API which aren't re-exported. Fix this by explicitly creating
bindings for each imported symbol.
@c42f c42f force-pushed the cjf/fix-logging-symbols branch from 006151b to 50e3eb2 Compare July 3, 2019 00:30
@c42f
Copy link
Member Author

c42f commented Jul 3, 2019

Tests added.

@StefanKarpinski
Copy link
Member

Any thoughts on who might review?

@KristofferC
Copy link
Member

@c42f
Copy link
Member Author

c42f commented Jul 3, 2019

Yes, I'm doing this for the same reasons as Pkg (I think).

It's a minor change though there are alternatives such as just exporting these parts of the public API. @oxinabox any thoughts?

@oxinabox
Copy link
Contributor

oxinabox commented Jul 4, 2019

On the one hand this is gross, and we should just fix tab-completion.

On the other hand, indeed I do indeed only access these things via Loghing. never CoreLogging

@c42f
Copy link
Member Author

c42f commented Jul 4, 2019

Fixing tab completion "properly" depends on having the right information in the runtime about the desired visibility of symbols. One way to give this information is just to create the bindings explicitly, as I've done here. It's gross but in a reasonably maintainable kind of way ;-)

The root of the issue is in the flags on jl_binding_t and how these are set when bindings are created:

julia/src/julia.h

Lines 456 to 466 in 61a0b04

typedef struct {
// not first-class
jl_sym_t *name;
jl_value_t *value;
jl_value_t *globalref; // cached GlobalRef for this binding
struct _jl_module_t *owner; // for individual imported bindings
uint8_t constp:1;
uint8_t exportp:1;
uint8_t imported:1;
uint8_t deprecated:2; // 0=not deprecated, 1=renamed, 2=moved to another package
} jl_binding_t;

From reading module_import_ in module.c, the imported flag is set from import statements, but not from using statements. So we could change REPL tab completion such that it uses names(mod; imported=true) and this would fix the problem for Logging in particular.

Perhaps this is a reasonable alternative; using import in source code rather than using could be seen as a sign that one will treating the binding as "native to the module".

@oxinabox
Copy link
Contributor

oxinabox commented Jul 4, 2019

Perhaps this is a reasonable alternative; using import in source code rather than using could be seen as a sign that one will treating the binding as "native to the module".

What about export?
the Logging stdlib reexports all of those

@fredrikekre fredrikekre merged commit 4d68846 into master Jul 4, 2019
@fredrikekre fredrikekre deleted the cjf/fix-logging-symbols branch July 4, 2019 11:50
@c42f
Copy link
Member Author

c42f commented Jul 4, 2019

It doesn't export handle_message and friends. The idea of not exporting those being that there's generally no reason you want to call them unless you're making a logging extension module.

Anyway, this will work for now, cheers for just going ahead with the merge.

Keno added a commit that referenced this pull request Mar 25, 2025
We now have `public` to indicate public symbols, which addresses
the original reason this hack was introduced. Semantically reverts #32473.
topolarity pushed a commit that referenced this pull request Mar 27, 2025
We now have `public` to indicate public symbols, which addresses the
original reason this hack was introduced. Semantically reverts #32473.
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 this pull request may close these issues.

5 participants