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

preserve structure of using and import statements in parser #25256

Merged
merged 1 commit into from
Dec 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 31 additions & 9 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,31 @@ function show_generator(io, ex, indent)
end
end

function show_import_path(io::IO, ex)
if !isa(ex, Expr)
print(io, ex)
elseif ex.head === :(:)
show_import_path(io, ex.args[1])
print(io, ": ")
for i = 2:length(ex.args)
if i > 2
print(io, ", ")
end
show_import_path(io, ex.args[i])
end
elseif ex.head === :(.)
print(io, ex.args[1])
for i = 2:length(ex.args)
if ex.args[i-1] != :(.)
print(io, '.')
end
print(io, ex.args[i])
end
else
show_unquoted(io, ex)
end
end

# TODO: implement interpolated strings
function show_unquoted(io::IO, ex::Expr, indent::Int, prec::Int)
head, args, nargs = ex.head, ex.args, length(ex.args)
Expand Down Expand Up @@ -1251,17 +1276,14 @@ function show_unquoted(io::IO, ex::Expr, indent::Int, prec::Int)

elseif head === :import || head === :using
print(io, head)
print(io, ' ')
first = true
for a = args
if first
print(io, ' ')
first = false
else
print(io, '.')
end
if a !== :.
print(io, a)
for a in args
if !first
print(io, ", ")
end
first = false
show_import_path(io, a)
end
elseif head === :meta && length(args) >= 2 && args[1] === :push_loc
print(io, "# meta: location ", join(args[2:end], " "))
Expand Down
12 changes: 6 additions & 6 deletions doc/src/devdocs/ast.md
Original file line number Diff line number Diff line change
Expand Up @@ -435,12 +435,12 @@ parses as `(macrocall (|.| Core '@doc) (line) "some docs" (= (call f x) (block x

| Input | AST |
|:------------------- |:-------------------------------------------- |
| `import a` | `(import a)` |
| `import a.b.c` | `(import a b c)` |
| `import ...a` | `(import . . . a)` |
| `import a.b, c.d` | `(toplevel (import a b) (import c d))` |
| `import Base: x` | `(import Base x)` |
| `import Base: x, y` | `(toplevel (import Base x) (import Base y))` |
| `import a` | `(import (. a))` |
| `import a.b.c` | `(import (. a b c))` |
| `import ...a` | `(import (. . . . a))` |
| `import a.b, c.d` | `(import (. a b) (. c d))` |
| `import Base: x` | `(import (: (. Base) (. x)))` |
| `import Base: x, y` | `(import (: (. Base) (. x) (. y)))` |
| `export a, b` | `(export a b)` |

### Numbers
Expand Down
3 changes: 2 additions & 1 deletion src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ jl_sym_t *polly_sym; jl_sym_t *inline_sym;
jl_sym_t *propagate_inbounds_sym; jl_sym_t *generated_sym;
jl_sym_t *generated_only_sym;
jl_sym_t *isdefined_sym; jl_sym_t *nospecialize_sym;
jl_sym_t *macrocall_sym;
jl_sym_t *macrocall_sym; jl_sym_t *colon_sym;
jl_sym_t *hygienicscope_sym;
jl_sym_t *escape_sym;
jl_sym_t *gc_preserve_begin_sym; jl_sym_t *gc_preserve_end_sym;
Expand Down Expand Up @@ -319,6 +319,7 @@ void jl_init_frontend(void)
structtype_sym = jl_symbol("struct_type");
toplevel_sym = jl_symbol("toplevel");
dot_sym = jl_symbol(".");
colon_sym = jl_symbol(":");
boundscheck_sym = jl_symbol("boundscheck");
inbounds_sym = jl_symbol("inbounds");
fastmath_sym = jl_symbol("fastmath");
Expand Down
15 changes: 5 additions & 10 deletions src/julia-parser.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1574,10 +1574,7 @@
(error "invalid \"export\" statement"))
`(export ,@es)))
((import using importall)
(let ((imports (parse-imports s word)))
(if (length= imports 1)
(car imports)
(cons 'toplevel imports))))
(parse-imports s word))
((do)
(error "invalid \"do\" syntax"))
(else (error "unhandled reserved word")))))))
Expand Down Expand Up @@ -1612,10 +1609,8 @@
(parse-comma-separated s (lambda (s)
(parse-import s word))))))
(if from
(map (lambda (x)
(cons (car x) (append (cdr first) (cdr x))))
rest)
(cons first rest))))
`(,word (|:| ,first ,@rest))
(list* word first rest))))

(define (parse-import-dots s)
(let loop ((l '())
Expand Down Expand Up @@ -1647,13 +1642,13 @@
(loop (cons (macrocall-to-atsym (parse-unary-prefix s)) path)))
((or (memv nxt '(#\newline #\; #\, :))
(eof-object? nxt))
`(,word ,@(reverse path)))
(cons '|.| (reverse path)))
((eqv? (string.sub (string nxt) 0 1) ".")
(take-token s)
(loop (cons (symbol (string.sub (string nxt) 1))
path)))
(else
`(,word ,@(reverse path)))))))
(cons '|.| (reverse path)))))))

;; parse comma-separated assignments, like "i=1:n,j=1:m,..."
(define (parse-comma-separated s what)
Expand Down
2 changes: 1 addition & 1 deletion src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1888,7 +1888,7 @@
(error (string "invalid " syntax-str " \"" (deparse el) "\""))))))))

(define (expand-forms e)
(if (or (atom? e) (memq (car e) '(quote inert top core globalref outerref line module toplevel ssavalue null meta)))
(if (or (atom? e) (memq (car e) '(quote inert top core globalref outerref line module toplevel ssavalue null meta using import importall export)))
e
(let ((ex (get expand-table (car e) #f)))
(if ex
Expand Down
2 changes: 1 addition & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ extern jl_sym_t *enter_sym; extern jl_sym_t *leave_sym;
extern jl_sym_t *exc_sym; extern jl_sym_t *new_sym;
extern jl_sym_t *compiler_temp_sym; extern jl_sym_t *foreigncall_sym;
extern jl_sym_t *const_sym; extern jl_sym_t *thunk_sym;
extern jl_sym_t *underscore_sym;
extern jl_sym_t *underscore_sym; extern jl_sym_t *colon_sym;
extern jl_sym_t *abstracttype_sym; extern jl_sym_t *primtype_sym;
extern jl_sym_t *structtype_sym;
extern jl_sym_t *global_sym; extern jl_sym_t *unused_sym;
Expand Down
137 changes: 102 additions & 35 deletions src/toplevel.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Sponsor Member

@StefanKarpinski StefanKarpinski Dec 30, 2017

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

Copy link
Sponsor Member Author

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.

Copy link
Sponsor Member

@StefanKarpinski StefanKarpinski Dec 30, 2017

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.

{
jl_sym_t *var = (jl_sym_t*)jl_array_ptr_ref(args, 0);
size_t i = 1;
Expand All @@ -429,7 +429,11 @@ static jl_module_t *eval_import_path(jl_module_t *from, jl_array_t *args, jl_sym
if (!jl_is_symbol(var))
jl_type_error(keyword, (jl_value_t*)jl_sym_type, (jl_value_t*)var);

if (var != dot_sym) {
if (from != NULL) {
m = from;
i = 0;
}
else if (var != dot_sym) {
// `A.B`: call the loader to obtain the root A in the current environment.
if (jl_core_module && var == jl_core_module->name) {
m = jl_core_module;
Expand All @@ -445,7 +449,7 @@ static jl_module_t *eval_import_path(jl_module_t *from, jl_array_t *args, jl_sym
}
else {
// `.A.B.C`: strip off leading dots by following parent links
m = from;
m = where;
while (1) {
if (i >= jl_array_len(args))
jl_error("invalid module path");
Expand All @@ -461,6 +465,8 @@ static jl_module_t *eval_import_path(jl_module_t *from, jl_array_t *args, jl_sym
var = (jl_sym_t*)jl_array_ptr_ref(args, i);
if (!jl_is_symbol(var))
jl_type_error(keyword, (jl_value_t*)jl_sym_type, (jl_value_t*)var);
if (var == dot_sym)
jl_errorf("invalid %s path: \".\" in identifier path", keyword);
if (i == jl_array_len(args)-1)
break;
m = (jl_module_t*)jl_eval_global_var(m, var);
Expand Down Expand Up @@ -522,6 +528,31 @@ static jl_module_t *deprecation_replacement_module(jl_module_t *parent, jl_sym_t
return NULL;
}

// in `import A.B: x, y, ...`, evaluate the `A.B` part if it exists
static jl_module_t *eval_import_from(jl_module_t *m, jl_expr_t *ex, const char *keyword)
{
if (jl_expr_nargs(ex) == 1 && jl_is_expr(jl_exprarg(ex, 0))) {
jl_expr_t *fr = (jl_expr_t*)jl_exprarg(ex, 0);
if (fr->head == colon_sym) {
if (jl_expr_nargs(fr) > 0 && jl_is_expr(jl_exprarg(fr, 0))) {
jl_expr_t *path = (jl_expr_t*)jl_exprarg(fr, 0);
if (((jl_expr_t*)path)->head == dot_sym) {
jl_sym_t *name = NULL;
jl_module_t *from = eval_import_path(m, NULL, path->args, &name, "import");
if (name != NULL) {
from = (jl_module_t*)jl_eval_global_var(from, name);
if (!jl_is_module(from))
jl_errorf("invalid %s path: \"%s\" does not name a module", keyword, jl_symbol_name(name));
}
return from;
}
}
jl_errorf("malformed \"%s:\" expression", keyword);
}
}
return NULL;
}

static jl_code_info_t *expr_to_code_info(jl_value_t *expr)
{
jl_code_info_t *src = jl_new_code_info_uninit();
Expand Down Expand Up @@ -569,50 +600,86 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *m, jl_value_t *e, int fast, int e
jl_sym_t *name = NULL;
jl_depwarn("`importall` is deprecated, use `using` or individual `import` statements instead",
(jl_value_t*)jl_symbol("importall"));
jl_module_t *import = eval_import_path(m, ex->args, &name, "importall");
if (name != NULL) {
import = (jl_module_t*)jl_eval_global_var(import, name);
if (!jl_is_module(import))
jl_errorf("invalid %s statement: name exists but does not refer to a module", jl_symbol_name(ex->head));
jl_module_t *from = eval_import_from(m, ex, "importall");
size_t i = 0;
if (from) {
i = 1;
ex = (jl_expr_t*)jl_exprarg(ex, 0);
}
for (; i < jl_expr_nargs(ex); i++) {
jl_value_t *a = jl_exprarg(ex, i);
if (jl_is_expr(a) && ((jl_expr_t*)a)->head == dot_sym) {
name = NULL;
jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "importall");
if (name != NULL) {
import = (jl_module_t*)jl_eval_global_var(import, name);
if (!jl_is_module(import))
jl_errorf("invalid %s statement: name exists but does not refer to a module", jl_symbol_name(ex->head));
}
jl_module_importall(m, import);
}
}
jl_module_importall(m, import);
return jl_nothing;
}
else if (ex->head == using_sym) {
jl_sym_t *name = NULL;
jl_module_t *import = eval_import_path(m, ex->args, &name, "using");
jl_module_t *u = import;
if (name != NULL)
u = (jl_module_t*)jl_eval_global_var(import, name);
if (jl_is_module(u)) {
jl_module_using(m, u);
if (m == jl_main_module && name == NULL) {
// TODO: for now, `using A` in Main also creates an explicit binding for `A`
// This will possibly be extended to all modules.
import_module(m, u);
}
jl_module_t *from = eval_import_from(m, ex, "using");
size_t i = 0;
if (from) {
i = 1;
ex = (jl_expr_t*)jl_exprarg(ex, 0);
}
else {
jl_module_t *replacement = deprecation_replacement_module(import, name);
if (replacement)
jl_module_using(m, replacement);
else
jl_module_use(m, import, name);
for (; i < jl_expr_nargs(ex); i++) {
jl_value_t *a = jl_exprarg(ex, i);
if (jl_is_expr(a) && ((jl_expr_t*)a)->head == dot_sym) {
name = NULL;
jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "using");
jl_module_t *u = import;
if (name != NULL)
u = (jl_module_t*)jl_eval_global_var(import, name);
if (jl_is_module(u)) {
jl_module_using(m, u);
if (m == jl_main_module && name == NULL) {
// TODO: for now, `using A` in Main also creates an explicit binding for `A`
// This will possibly be extended to all modules.
import_module(m, u);
}
}
else {
jl_module_t *replacement = deprecation_replacement_module(import, name);
if (replacement)
jl_module_using(m, replacement);
else
jl_module_use(m, import, name);
}
}
}
return jl_nothing;
}
else if (ex->head == import_sym) {
jl_sym_t *name = NULL;
jl_module_t *import = eval_import_path(m, ex->args, &name, "import");
if (name == NULL) {
import_module(m, import);
jl_module_t *from = eval_import_from(m, ex, "import");
size_t i = 0;
if (from) {
i = 1;
ex = (jl_expr_t*)jl_exprarg(ex, 0);
}
else {
jl_module_t *replacement = deprecation_replacement_module(import, name);
if (replacement)
import_module(m, replacement);
else
jl_module_import(m, import, name);
for (; i < jl_expr_nargs(ex); i++) {
jl_value_t *a = jl_exprarg(ex, i);
if (jl_is_expr(a) && ((jl_expr_t*)a)->head == dot_sym) {
name = NULL;
jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "import");
if (name == NULL) {
import_module(m, import);
}
else {
jl_module_t *replacement = deprecation_replacement_module(import, name);
if (replacement)
import_module(m, replacement);
else
jl_module_import(m, import, name);
}
}
}
return jl_nothing;
}
Expand Down
11 changes: 9 additions & 2 deletions stdlib/Distributed/src/macros.jl
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,16 @@ end
extract_imports!(imports, x) = imports
function extract_imports!(imports, ex::Expr)
if Meta.isexpr(ex, (:import, :using))
return push!(imports, ex.args[1])
m = ex.args[1]
if isa(m, Expr) && m.head === :(:)
push!(imports, m.args[1].args[1])
else
for a in ex.args
push!(imports, a.args[1])
end
end
elseif Meta.isexpr(ex, :let)
return extract_imports!(imports, ex.args[2])
extract_imports!(imports, ex.args[2])
elseif Meta.isexpr(ex, (:toplevel, :block))
for i in eachindex(ex.args)
extract_imports!(imports, ex.args[i])
Expand Down
3 changes: 3 additions & 0 deletions stdlib/Distributed/test/distributed_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import Distributed: launch, manage

include(joinpath(Sys.BINDIR, "..", "share", "julia", "test", "testenv.jl"))

@test Distributed.extract_imports(:(begin; import Foo, Bar; let; using Baz; end; end)) ==
[:Foo, :Bar, :Baz]

# Test a few "remote" invocations when no workers are present
@test remote(myid)() == 1
@test pmap(identity, 1:100) == [1:100...]
Expand Down
4 changes: 0 additions & 4 deletions test/meta.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ show_sexpr(ioB,:(1+1))

show_sexpr(ioB,QuoteNode(1),1)

using Distributed
@test Distributed.extract_imports(:(begin; import Foo, Bar; let; using Baz; end; end)) ==
[:Foo, :Bar, :Baz]

# test base/expr.jl
baremodule B
eval = 0
Expand Down
Loading