-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
preserve structure of using
and import
statements in parser
#25256
Conversation
63f09f7
to
9711d34
Compare
This allows round-trip printing and more flexibility in how the statements are interpreted.
9711d34
to
4ad46c1
Compare
@@ -420,7 +420,7 @@ static jl_module_t *call_require(jl_sym_t *var) | |||
// either: | |||
// - sets *name and returns the module to import *name from | |||
// - sets *name to NULL and returns a module to import | |||
static jl_module_t *eval_import_path(jl_module_t *from, jl_array_t *args, jl_sym_t **name, const char *keyword) | |||
static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from, jl_array_t *args, jl_sym_t **name, const char *keyword) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to pre-existing unfortunate naming, it is now quite unclear to me what the roles of these two modules are and are intended to be. Is where
the module in which the statement appears? Is from
the module from which the bindings are being imported? The name from
is used in many other places in this file to indicate the module in which an import
or using
statement appears. Or at least that was my understanding, and my attempts to change code loading appear to bear it out. It might be nice if there were comments explaining what the arguments mean. cc @vtjnash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is where the module in which the statement appears? Is from the module from which the bindings are being imported?
Yes. No need to copy Jameson here; I believe this is mostly my code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The from
name is used heavily in the precompilation code to mean the opposite (i.e. the module you are loading code into), I'm cc:ing him to confirm that it has the opposite meaning there. So much so that the branch where I've been working on this is called sk/reqfrom; that branch conflicts with this and when I try to rebase onto this, the naming becomes a bit of a problem.
This allows round-trip printing and more flexibility in how the statements are interpreted.
This allows round-trip printing and more flexibility in how the statements are interpreted, e.g. letting us treat
import A.x
andimport A: x
differently if we want to. Though it is a bit harder on the interpreter.Part of #8000, and also in the spirit of #21774.