Merge pull request #33864 from JuliaLang/jb/softscope

bring back v0.6 scope rules in the REPL

Author: JeffBezanson

NEWS.md

@@ -8,6 +8,19 @@ New language features
 Language changes
 ----------------
 
+* 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 ([#28789], [#33864]).
+
+* Outside of the REPL (e.g. in a file), assigning to a variable within a top-level scope
+  block is considered ambiguous if a global variable with the same name exists.
+  A warning is given if that happens, to alert you that the code will work differently
+  than in the REPL.
+  A new command line option `--warn-scope` controls this warning ([#33864]).
+
 * Converting arbitrary tuples to `NTuple`, e.g. `convert(NTuple, (1, ""))` now gives an error,
   where it used to be incorrectly allowed. This is because `NTuple` refers only to homogeneous
   tuples (this meaning has not changed) ([#34272]).
@@ -19,7 +32,6 @@ Language changes
   (in addition to the one that enters the help mode) to see the full
   docstring. ([#25930])
 
-
 Multi-threading changes
 -----------------------
 

base/options.jl

@@ -41,6 +41,7 @@ struct JLOptions
     outputji::Ptr{UInt8}
     output_code_coverage::Ptr{UInt8}
     incremental::Int8
+    warn_scope::Int8
 end
 
 # This runs early in the sysimage != is not defined yet

doc/src/manual/variables-and-scoping.md

@@ -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 }
 };
 
@@ -227,6 +251,7 @@ static void jl_init_ast_ctx(jl_ast_context_t *ast_ctx) JL_NOTSAFEPOINT
     ctx->task = NULL;
     ctx->module = NULL;
     set(symbol(fl_ctx, "*depwarn-opt*"), fixnum(jl_options.depwarn));
+    set(symbol(fl_ctx, "*scopewarn-opt*"), fixnum(jl_options.warn_scope));
 }
 
 // There should be no GC allocation while holding this lock
@@ -859,9 +884,10 @@ jl_value_t *jl_parse_eval_all(const char *fname,
                 }
                 // expand non-final expressions in statement position (value unused)
                 expression =
-                    fl_applyn(fl_ctx, 3,
-                              symbol_value(symbol(fl_ctx, iscons(cdr_(ast)) ? "jl-expand-to-thunk-stmt" : "jl-expand-to-thunk")),
-                              expression, symbol(fl_ctx, jl_filename), fixnum(jl_lineno));
+                    fl_applyn(fl_ctx, 4,
+                              symbol_value(symbol(fl_ctx, "jl-expand-to-thunk-warn")),
+                              expression, symbol(fl_ctx, jl_filename), fixnum(jl_lineno),
+                              iscons(cdr_(ast)) ? fl_ctx->T : fl_ctx->F);
             }
             jl_get_ptls_states()->world_age = jl_world_counter;
             form = scm_to_julia(fl_ctx, expression, inmodule);
@@ -1171,6 +1197,13 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand1(jl_value_t *expr, jl_module_t *inmodule
     return expr;
 }
 
+// Lower an expression tree into Julia's intermediate-representation.
+JL_DLLEXPORT jl_value_t *jl_expand(jl_value_t *expr, jl_module_t *inmodule)
+{
+    return jl_expand_with_loc(expr, inmodule, "none", 0);
+}
+
+// Lowering, with starting program location specified
 JL_DLLEXPORT jl_value_t *jl_expand_with_loc(jl_value_t *expr, jl_module_t *inmodule,
                                             const char *file, int line)
 {
@@ -1183,10 +1216,25 @@ JL_DLLEXPORT jl_value_t *jl_expand_with_loc(jl_value_t *expr, jl_module_t *inmod
     return expr;
 }
 
-// Lower an expression tree into Julia's intermediate-representation.
-JL_DLLEXPORT jl_value_t *jl_expand(jl_value_t *expr, jl_module_t *inmodule)
+// Same as the above, but printing warnings when applicable
+JL_DLLEXPORT jl_value_t *jl_expand_with_loc_warn(jl_value_t *expr, jl_module_t *inmodule,
+                                                 const char *file, int line)
 {
-    return jl_expand_with_loc(expr, inmodule, "none", 0);
+    JL_TIMING(LOWERING);
+    JL_GC_PUSH1(&expr);
+    expr = jl_copy_ast(expr);
+    expr = jl_expand_macros(expr, inmodule, NULL, 0);
+    jl_ast_context_t *ctx = jl_ast_ctx_enter();
+    fl_context_t *fl_ctx = &ctx->fl;
+    JL_AST_PRESERVE_PUSH(ctx, old_roots, inmodule);
+    value_t arg = julia_to_scm(fl_ctx, expr);
+    value_t e = fl_applyn(fl_ctx, 4, symbol_value(symbol(fl_ctx, "jl-expand-to-thunk-warn")), arg,
+                          symbol(fl_ctx, file), fixnum(line), fl_ctx->F);
+    expr = scm_to_julia(fl_ctx, e, inmodule);
+    JL_AST_PRESERVE_POP(ctx, old_roots);
+    jl_ast_ctx_leave(ctx);
+    JL_GC_POP();
+    return expr;
 }
 
 // expand in a context where the expression value is unused

src/ast.c

@@ -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
@@ -128,16 +133,38 @@
            (begin0 (expand-toplevel-expr-- e file line)
                    (set! *in-expand* last))))))
 
-; expand a piece of raw surface syntax to an executable thunk
-(define (jl-expand-to-thunk expr file line)
+;; used to collect warnings during lowering, which are usually discarded
+;; unless logging is requested
+(define lowering-warning (lambda lst (void)))
+
+;; expand a piece of raw surface syntax to an executable thunk
+
+(define (expand-to-thunk- expr file line)
   (error-wrap (lambda ()
                 (expand-toplevel-expr expr file line))))
 
+(define (expand-to-thunk-stmt- expr file line)
+  (expand-to-thunk- (if (toplevel-only-expr? expr)
+                        expr
+                        `(block ,expr (null)))
+                    file line))
+
+(define (jl-expand-to-thunk-warn expr file line stmt)
+  (let ((warnings '()))
+    (with-bindings
+     ((lowering-warning (lambda lst (set! warnings (cons lst warnings)))))
+     (begin0
+      (if stmt
+          (expand-to-thunk-stmt- expr file line)
+          (expand-to-thunk- expr file line))
+      (for-each (lambda (args) (apply julia-logmsg args))
+                (reverse warnings))))))
+
+(define (jl-expand-to-thunk expr file line)
+  (expand-to-thunk- expr file line))
+
 (define (jl-expand-to-thunk-stmt expr file line)
-  (jl-expand-to-thunk (if (toplevel-only-expr? expr)
-                          expr
-                          `(block ,expr (null)))
-                      file line))
+  (expand-to-thunk-stmt- expr file line))
 
 (define (jl-expand-macroscope expr)
   (error-wrap (lambda ()
@@ -215,6 +242,8 @@
           (if (equal? instead "") ""
               (string #\newline "Use `" instead "` instead."))))
 
+(define *scopewarn-opt* 1)
+
 ; Corresponds to --depwarn 0="no", 1="yes", 2="error"
 (define *depwarn-opt* 1)
 

src/jlfrontend.scm

@@ -71,7 +71,8 @@ jl_options_t jl_options = { 0,    // quiet
                             NULL, // output-ji
                             NULL,    // output-code_coverage
                             0, // incremental
-                            0 // image_file_specified
+                            0, // image_file_specified
+                            JL_OPTIONS_WARN_SCOPE_ON  // ambiguous scope warning
 };
 
 static const char usage[] = "julia [switches] -- [programfile] [args...]\n";
@@ -109,7 +110,8 @@ static const char opts[]  =
 
     // error and warning options
     " --depwarn={yes|no|error}  Enable or disable syntax and method deprecation warnings (\"error\" turns warnings into errors)\n"
-    " --warn-overwrite={yes|no} Enable or disable method overwrite warnings\n\n"
+    " --warn-overwrite={yes|no} Enable or disable method overwrite warnings\n"
+    " --warn-scope={yes|no}     Enable or disable warning for ambiguous top-level scope\n\n"
 
     // code generation options
     " -C, --cpu-target <target> Limit usage of CPU features up to <target>; set to \"help\" to see the available options\n"
@@ -169,6 +171,7 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
            opt_output_bc,
            opt_depwarn,
            opt_warn_overwrite,
+           opt_warn_scope,
            opt_inline,
            opt_polly,
            opt_trace_compile,
@@ -225,6 +228,7 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
         { "output-incremental",required_argument, 0, opt_incremental },
         { "depwarn",         required_argument, 0, opt_depwarn },
         { "warn-overwrite",  required_argument, 0, opt_warn_overwrite },
+        { "warn-scope",      required_argument, 0, opt_warn_scope },
         { "inline",          required_argument, 0, opt_inline },
         { "polly",           required_argument, 0, opt_polly },
         { "trace-compile",   required_argument, 0, opt_trace_compile },
@@ -554,7 +558,15 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
             else if (!strcmp(optarg,"no"))
                 jl_options.warn_overwrite = JL_OPTIONS_WARN_OVERWRITE_OFF;
             else
-                jl_errorf("julia: invalid argument to --warn-overwrite={yes|no|} (%s)", optarg);
+                jl_errorf("julia: invalid argument to --warn-overwrite={yes|no} (%s)", optarg);
+            break;
+        case opt_warn_scope:
+            if (!strcmp(optarg,"yes"))
+                jl_options.warn_scope = JL_OPTIONS_WARN_SCOPE_ON;
+            else if (!strcmp(optarg,"no"))
+                jl_options.warn_scope = JL_OPTIONS_WARN_SCOPE_OFF;
+            else
+                jl_errorf("julia: invalid argument to --warn-scope={yes|no} (%s)", optarg);
             break;
         case opt_inline:
             if (!strcmp(optarg,"yes"))