bring back v0.6 scope rules in the REPL

warn when an implicit local at the toplevel shadows a global

fixes #28789

NEWS.md

@@ -12,6 +12,13 @@ Language changes
   where it used to be incorrectly allowed. This is because `NTuple` refers only to homogeneous
   tuples (this meaning has not changed) ([#34272]).
 
+* The interactive REPL now uses "soft scope" for top-level expressions: an assignment inside a
+  scope block such as a `for` loop automatically assigns to a global variable if one has been
+  defined already. This matches the behavior of Julia versions 0.6 and prior, as well as
+  [IJulia](https://github.com/JuliaLang/IJulia.jl).
+  Note that this only affects expressions interactively typed or pasted directly into the
+  default REPL.
+
 Multi-threading changes
 -----------------------
 

src/ast.c

@@ -131,13 +131,34 @@ static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mo
 static value_t julia_to_scm(fl_context_t *fl_ctx, jl_value_t *v);
 static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, struct macroctx_stack *macroctx, int onelevel);
 
+value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
+{
+    // tells whether a var is defined in and *by* the current module
+    argcount(fl_ctx, "defined-julia-global", nargs, 1);
+    (void)tosymbol(fl_ctx, args[0], "defined-julia-global");
+    jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
+    jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0]));
+    jl_binding_t *b = jl_get_module_binding(ctx->module, var);
+    return (b != NULL && b->owner == ctx->module) ? fl_ctx->T : fl_ctx->F;
+}
+
 value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
 {
     jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
     assert(ctx->module);
     return fixnum(jl_module_next_counter(ctx->module));
 }
 
+value_t fl_julia_current_file(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
+{
+    return symbol(fl_ctx, jl_filename);
+}
+
+value_t fl_julia_current_line(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
+{
+    return fixnum(jl_lineno);
+}
+
 // Check whether v is a scalar for purposes of inlining fused-broadcast
 // arguments when lowering; should agree with broadcast.jl on what is a
 // scalar.  When in doubt, return false, since this is only an optimization.
@@ -197,9 +218,12 @@ value_t fl_julia_logmsg(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
 }
 
 static const builtinspec_t julia_flisp_ast_ext[] = {
+    { "defined-julia-global", fl_defined_julia_global },
     { "current-julia-module-counter", fl_current_module_counter },
     { "julia-scalar?", fl_julia_scalar },
     { "julia-logmsg", fl_julia_logmsg },
+    { "julia-current-file", fl_julia_current_file },
+    { "julia-current-line", fl_julia_current_line },
     { NULL, NULL }
 };
 

src/jlfrontend.scm

@@ -29,6 +29,11 @@
            '(error "malformed expression"))))
    thk))
 
+;; this is overwritten when we run in actual julia
+(define (defined-julia-global v) #f)
+(define (julia-current-file) 'none)
+(define (julia-current-line) 0)
+
 ;; parser entry points
 
 ;; parse one expression (if greedy) or atom, returning end position

src/julia-syntax.scm

@@ -2447,19 +2447,30 @@
 (define (find-local-def-decls e) (find-decls 'local-def e))
 (define (find-global-decls e) (find-decls 'global e))
 
+(define (find-softscope e)
+  (expr-contains-p
+   (lambda (x) (and (pair? x) (eq? (car x) 'softscope) x))
+   e
+   (lambda (x) (not (and (pair? x)
+                         (memq (car x) '(lambda scope-block module toplevel)))))))
+
 (define (check-valid-name e)
   (or (valid-name? e)
       (error (string "invalid identifier name \"" e "\""))))
 
-(define (make-scope (lam #f) (args '()) (locals '()) (globals '()) (sp '()) (renames '()) (prev #f))
-  (vector lam args locals globals sp renames prev))
+(define (make-scope (lam #f) (args '()) (locals '()) (globals '()) (sp '()) (renames '()) (prev #f) (soft? #f)
+                    (implicit-globals '()) (warn-vars #f))
+  (vector lam args locals globals sp renames prev soft? implicit-globals warn-vars))
 (define (scope:lam s)     (aref s 0))
 (define (scope:args s)    (aref s 1))
 (define (scope:locals s)  (aref s 2))
 (define (scope:globals s) (aref s 3))
 (define (scope:sp s)      (aref s 4))
 (define (scope:renames s) (aref s 5))
 (define (scope:prev s)    (aref s 6))
+(define (scope:soft? s)   (aref s 7))
+(define (scope:implicit-globals s) (aref s 8))
+(define (scope:warn-vars s) (aref s 9))
 
 (define (var-kind var scope)
   (if scope
@@ -2472,6 +2483,14 @@
 
 (define (in-scope? var scope) (not (eq? (var-kind var scope) 'none)))
 
+(define (warn-var?! v scope)
+  (and scope
+       (let ((w (scope:warn-vars scope)))
+         (if (and w (has? w v))
+             (begin (del! w v)
+                    #t)
+             (warn-var?! v (scope:prev scope))))))
+
 (define (all-local-names scope)
   (define (all-lists s)
     (if s
@@ -2480,7 +2499,7 @@
   (apply append (all-lists scope)))
 
 ;; returns lambdas in the form (lambda (args...) (locals...) body)
-(define (resolve-scopes- e scope (sp '()))
+(define (resolve-scopes- e scope (sp '()) (loc #f))
   (cond ((symbol? e)
          (let lookup ((scope scope))
            (if scope
@@ -2509,6 +2528,8 @@
          (if (not (in-scope? (cadr e) scope))
              (error "no outer local variable declaration exists for \"for outer\""))
          '(null))
+        ((eq? (car e) 'softscope)
+         '(null))
         ((eq? (car e) 'locals)
          (let* ((names (filter (lambda (v)
                                  (and (not (gensym? v))
@@ -2537,20 +2558,32 @@
          (let* ((blok            (cadr e)) ;; body of scope-block expression
                 (lam             (scope:lam scope))
                 (argnames        (lam:vars lam))
+                (toplevel?       (and (null? argnames) (eq? e (lam:body lam))))
                 (current-locals  (caddr lam)) ;; locals created so far in our lambda
                 (globals         (find-global-decls blok))
+                (assigned        (find-assigned-vars blok))
                 (locals-def      (find-local-def-decls blok))
                 (local-decls     (find-local-decls blok))
-                (toplevel?       (and (null? argnames) (eq? e (lam:body lam))))
+                (soft?           (and (null? argnames)
+                                      (let ((ss (find-softscope blok)))
+                                        (cond ((not ss) (scope:soft? scope))
+                                              ((equal? (cadr ss) '(true))  #t)
+                                              ((equal? (cadr ss) '(false)) #f)
+                                              (else (scope:soft? scope))))))
+                (nonloc-assigned (filter (lambda (v) (and (not (memq v locals-def))
+                                                          (not (memq v local-decls))))
+                                         assigned))
+                (implicit-globals (if toplevel? nonloc-assigned '()))
                 (implicit-locals
                  (filter (if toplevel?
                              ;; make only assigned gensyms implicitly local at top level
                              some-gensym?
                              (lambda (v) (and (memq (var-kind v scope) '(none static-parameter))
-                                              (not (memq v locals-def))
-                                              (not (memq v local-decls))
+                                              (not (and soft?
+                                                        (or (memq v (scope:implicit-globals scope))
+                                                            (defined-julia-global v))))
                                               (not (memq v globals)))))
-                         (find-assigned-vars blok)))
+                         nonloc-assigned))
                 (locals-nondef   (delete-duplicates (append local-decls implicit-locals)))
                 (need-rename?    (lambda (vars)
                                    (filter (lambda (v) (or (memq v current-locals) (in-scope? v scope)))
@@ -2561,7 +2594,22 @@
                 (renamed         (map named-gensy need-rename))
                 (renamed-def     (map named-gensy need-rename-def))
                 (newnames        (append (diff locals-nondef need-rename) renamed))
-                (newnames-def    (append (diff locals-def need-rename-def) renamed-def)))
+                (newnames-def    (append (diff locals-def need-rename-def) renamed-def))
+                (warn-vars
+                 (and (not toplevel?) (null? argnames) (not soft?)
+                      (let ((vars (filter (lambda (v)
+                                            (and (or (memq v (scope:implicit-globals scope))
+                                                     (defined-julia-global v))
+                                                 (eq? (var-kind v scope) 'none)
+                                                 (not (memq v globals))))
+                                          nonloc-assigned)))
+                        (if (pair? vars)
+                            (let ((t (table)))
+                              (for-each (lambda (v) (put! t v #t))
+                                        vars)
+                              t)
+                            #f)))))
+
            (for-each (lambda (v)
                        (if (or (memq v locals-def) (memq v local-decls))
                            (error (string "variable \"" v "\" declared both local and global"))))
@@ -2591,27 +2639,51 @@
                                           '()
                                           (append (map cons need-rename renamed)
                                                   (map cons need-rename-def renamed-def))
-                                          scope)))
+                                          scope
+                                          (and soft? (null? argnames))
+                                          (if toplevel?
+                                              implicit-globals
+                                              (scope:implicit-globals scope))
+                                          warn-vars)
+                              '()
+                              loc))
             (append! (map (lambda (v) `(local ,v)) newnames)
                      (map (lambda (v) `(local-def ,v)) newnames-def)))
            ))
         ((eq? (car e) 'module)
          (error "\"module\" expression not at top level"))
         ((eq? (car e) 'break-block)
          `(break-block ,(cadr e) ;; ignore type symbol of break-block expression
-                       ,(resolve-scopes- (caddr e) scope))) ;; body of break-block expression
+                       ,(resolve-scopes- (caddr e) scope '() loc))) ;; body of break-block expression
         ((eq? (car e) 'with-static-parameters)
          `(with-static-parameters
-           ,(resolve-scopes- (cadr e) scope (cddr e))
+           ,(resolve-scopes- (cadr e) scope (cddr e) loc)
            ,@(cddr e)))
         ((and (eq? (car e) 'method) (length> e 2))
          `(method
            ,(resolve-scopes- (cadr   e) scope)
            ,(resolve-scopes- (caddr  e) scope)
            ,(resolve-scopes- (cadddr e) scope (method-expr-static-parameters e))))
         (else
+         (if (and (eq? (car e) '=) (symbol? (cadr e))
+                  scope (null? (lam:vars (scope:lam scope)))
+                  (warn-var?! (cadr e) scope))
+             (let* ((v    (cadr e))
+                    (loc  (extract-line-file loc))
+                    (line (if (= (car loc) 0) (julia-current-line) (car loc)))
+                    (file (if (eq? (cadr loc) 'none) (julia-current-file) (cadr loc))))
+               (julia-logmsg
+                1000 'warn (symbol (string file line)) file line
+                (string "Assignment to `" v "` in top-level block is ambiguous "
+                        "because an outer global binding by the same name already exists."
+                        " Use `global " v "` to assign to the outer global `" v
+                        "` variable or use `local " v "` to force a new "
+                        "local by the same name."))))
          (cons (car e)
-               (map (lambda (x) (resolve-scopes- x scope))
+               (map (lambda (x)
+                      (if (linenum? x)
+                          (set! loc x))
+                      (resolve-scopes- x scope '() loc))
                     (cdr e))))))
 
 (define (resolve-scopes e) (resolve-scopes- e #f))

stdlib/REPL/src/REPL.jl

@@ -73,6 +73,24 @@ mutable struct REPLBackend
         new(repl_channel, response_channel, in_eval)
 end
 
+function softscope!(ex)
+    if ex isa Expr
+        h = ex.head
+        if h === :toplevel
+            for i = 1:length(ex.args)
+                ex.args[i] = softscope!(ex.args[i])
+            end
+        elseif h in (:meta, :import, :using, :export, :module, :error, :incomplete, :thunk)
+            return ex
+        else
+            return Expr(:block, Expr(:softscope, true), ex)
+        end
+    end
+    return ex
+end
+
+const repl_ast_transforms = Any[softscope!]
+
 function eval_user_input(@nospecialize(ast), backend::REPLBackend)
     lasterr = nothing
     Base.sigatomic_begin()
@@ -83,6 +101,9 @@ function eval_user_input(@nospecialize(ast), backend::REPLBackend)
                 put!(backend.response_channel, (lasterr,true))
             else
                 backend.in_eval = true
+                for xf in repl_ast_transforms
+                    ast = xf(ast)
+                end
                 value = Core.eval(Main, ast)
                 backend.in_eval = false
                 # note: use jl_set_global to make sure value isn't passed through `expand`

test/syntax.jl

@@ -1994,3 +1994,73 @@ end
     end
     pop = 1
 end == 1
+
+# optional soft scope: #28789, #33864
+
+@test @eval begin
+    $(Expr(:softscope, true))
+    x28789 = 0   # new global included in same expression
+    for i = 1:2
+        x28789 += i
+    end
+    x28789
+end == 3
+
+y28789 = 1  # new global defined in separate top-level input
+@eval begin
+    $(Expr(:softscope, true))
+    for i = 1:10
+        y28789 += i
+    end
+end
+@test y28789 == 56
+
+@eval begin
+    $(Expr(:softscope, true))
+    for i = 10:10
+        z28789 = i
+    end
+    @test z28789 == 10
+    z28789 = 0  # new global assigned after loop but in same soft scope
+end
+
+@eval begin
+    $(Expr(:softscope, true))
+    let y28789 = 0  # shadowing with let
+        y28789 = 1
+    end
+end
+@test y28789 == 56
+
+@eval begin
+    $(Expr(:softscope, true))
+    let y28789 = 0
+        let x = 2
+            let y = 3
+                z28789 = 42  # assign to global despite several lets
+            end
+        end
+    end
+end
+@test z28789 == 42
+
+@eval begin
+    $(Expr(:softscope, true))
+    let x = 0
+        ww28789 = 88  # not global
+        let y = 3
+            ww28789 = 89
+        end
+        @test ww28789 == 89
+    end
+end
+@test !@isdefined(ww28789)
+
+@eval begin
+    $(Expr(:softscope, true))
+    function f28789()
+        z28789 = 43
+    end
+    f28789()
+end
+@test z28789 == 42