deprecate for loop vars that overwrite outer vars (#22314)

Author: JeffBezanson

base/deprecated.jl

@@ -90,7 +90,8 @@ function firstcaller(bt::Array{Ptr{Void},1}, funcsyms)
     lkup = StackTraces.UNKNOWN
     for frame in bt
         lkups = StackTraces.lookup(frame)
-        for lkup in lkups
+        for l in lkups
+            lkup = l
             if lkup == StackTraces.UNKNOWN
                 continue
             end
@@ -1067,7 +1068,8 @@ function partial_linear_indexing_warning_lookup(nidxs_remaining)
         caller = StackTraces.UNKNOWN
         for frame in bt
             lkups = StackTraces.lookup(frame)
-            for caller in lkups
+            for c_ in lkups
+                caller = c_
                 if caller == StackTraces.UNKNOWN
                     continue
                 end

base/docs/utils.jl

@@ -289,7 +289,8 @@ function levsort(search, candidates)
     scores = map(cand -> (levenshtein(search, cand), -fuzzyscore(search, cand)), candidates)
     candidates = candidates[sortperm(scores)]
     i = 0
-    for i = 1:length(candidates)
+    for i_ = 1:length(candidates)
+        i = i_
         levenshtein(search, candidates[i]) > 3 && break
     end
     return candidates[1:i]
@@ -328,7 +329,8 @@ printmatches(args...; cols = displaysize(STDOUT)[2]) = printmatches(STDOUT, args
 function print_joined_cols(io::IO, ss, delim = "", last = delim; cols = displaysize(io)[2])
     i = 0
     total = 0
-    for i = 1:length(ss)
+    for i_ = 1:length(ss)
+        i = i_
         total += length(ss[i])
         total + max(i-2,0)*length(delim) + (i>1 ? 1 : 0)*length(last) > cols && (i-=1; break)
     end

base/inference.jl

@@ -3515,9 +3515,9 @@ end
 function type_annotate!(sv::InferenceState)
     # remove all unused ssa values
     gt = sv.src.ssavaluetypes
-    for i = 1:length(gt)
-        if gt[i] === NF
-            gt[i] = Union{}
+    for j = 1:length(gt)
+        if gt[j] === NF
+            gt[j] = Union{}
         end
     end
 
@@ -3568,9 +3568,9 @@ function type_annotate!(sv::InferenceState)
     end
 
     # finish marking used-undef variables
-    for i = 1:nslots
-        if undefs[i]
-            src.slotflags[i] |= Slot_UsedUndef
+    for j = 1:nslots
+        if undefs[j]
+            src.slotflags[j] |= Slot_UsedUndef
         end
     end
     nothing
@@ -4572,10 +4572,10 @@ function inlineable(f::ANY, ft::ANY, e::Expr, atypes::Vector{Any}, sv::Inference
     if !isempty(stmts) && !propagate_inbounds
         # avoid redundant inbounds annotations
         s_1, s_end = stmts[1], stmts[end]
-        i = 2
-        while length(stmts) > i && ((isa(s_1,Expr)&&s_1.head===:line) || isa(s_1,LineNumberNode))
-            s_1 = stmts[i]
-            i += 1
+        si = 2
+        while length(stmts) > si && ((isa(s_1,Expr)&&s_1.head===:line) || isa(s_1,LineNumberNode))
+            s_1 = stmts[si]
+            si += 1
         end
         if isa(s_1, Expr) && s_1.head === :inbounds && s_1.args[1] === false &&
             isa(s_end, Expr) && s_end.head === :inbounds && s_end.args[1] === :pop

base/intfuncs.jl

@@ -801,9 +801,8 @@ end
 
 function factorial(n::Integer)
     n < 0 && throw(DomainError())
-    local f::typeof(n*n), i::typeof(n*n)
-    f = 1
-    for i = 2:n
+    f::typeof(n*n) = 1
+    for i::typeof(n*n) = 2:n
         f *= i
     end
     return f

base/linalg/triangular.jl

@@ -1833,7 +1833,9 @@ function logm(A0::UpperTriangular{T}) where T<:Union{Float64,Complex{Float64}}
         d4 = norm(AmI^4, 1)^(1/4)
         alpha3 = max(d3, d4)
         if alpha3 <= theta[tmax]
-            for j = 3:tmax
+            local j
+            for j_ = 3:tmax
+                j = j_
                 if alpha3 <= theta[j]
                     break
                 end
@@ -1853,7 +1855,8 @@ function logm(A0::UpperTriangular{T}) where T<:Union{Float64,Complex{Float64}}
             eta = min(alpha3, alpha4)
             if eta <= theta[tmax]
                 j = 0
-                for j = 6:tmax
+                for j_ = 6:tmax
+                    j = j_
                     if eta <= theta[j]
                         m = j
                         break
@@ -2032,7 +2035,9 @@ function invsquaring(A0::UpperTriangular, theta)
         d4 = norm(AmI^4, 1)^(1/4)
         alpha3 = max(d3, d4)
         if alpha3 <= theta[tmax]
-            for j = 3:tmax
+            local j
+            for j_ = 3:tmax
+                j = j_
                 if alpha3 <= theta[j]
                     break
                 elseif alpha3 / 2 <= theta[5] && p < 2
@@ -2056,7 +2061,8 @@ function invsquaring(A0::UpperTriangular, theta)
             eta = min(alpha3, alpha4)
             if eta <= theta[tmax]
                 j = 0
-                for j = 6:tmax
+                for j_ = 6:tmax
+                    j = j_
                     if eta <= theta[j]
                         m = j
                         break

base/markdown/Markdown.jl

@@ -56,8 +56,8 @@ macro doc_str(s::AbstractString, t...)
     docexpr(__source__, __module__, s, t...)
 end
 
-function Base.display(d::Base.REPL.REPLDisplay, md::Vector{MD})
-    for md in md
+function Base.display(d::Base.REPL.REPLDisplay, mds::Vector{MD})
+    for md in mds
         display(d, md)
     end
 end

base/markdown/render/latex.jl

@@ -46,8 +46,8 @@ function latexinline(io::IO, code::Code)
 end
 
 function latex(io::IO, md::Paragraph)
-    for md in md.content
-        latexinline(io, md)
+    for mdc in md.content
+        latexinline(io, mdc)
     end
     println(io)
     println(io)

base/markdown/render/rst.jl

@@ -90,8 +90,8 @@ end
 
 function rst(io::IO, l::LaTeX)
     println(io, ".. math::\n")
-    for l in lines(l.formula)
-        println(io, "    ", l)
+    for line in lines(l.formula)
+        println(io, "    ", line)
     end
 end
 

base/multidimensional.jl

@@ -812,9 +812,9 @@ end
 
 @noinline function _accumulate!(op, B, A, R1, ind, R2)
     # Copy the initial element in each 1d vector along dimension `axis`
-    i = first(ind)
+    ii = first(ind)
     @inbounds for J in R2, I in R1
-        B[I, i, J] = A[I, i, J]
+        B[I, ii, J] = A[I, ii, J]
     end
     # Accumulate
     @inbounds for J in R2, i in first(ind)+1:last(ind), I in R1

base/printf.jl

@@ -57,7 +57,8 @@ function parse(s::AbstractString)
     i = 1
     while i < length(list)
         if isa(list[i],AbstractString)
-            for j = i+1:length(list)
+            for j_ = i+1:length(list)
+                j = j_
                 if !isa(list[j],AbstractString)
                     j -= 1
                     break

base/sparse/linalg.jl

@@ -301,8 +301,8 @@ function A_rdiv_B!(A::SparseMatrixCSC{T}, D::Diagonal{T}) where T
         if iszero(ddj)
             throw(LinAlg.SingularException(j))
         end
-        for k in nzrange(A, j)
-            nonz[k] /= ddj
+        for i in nzrange(A, j)
+            nonz[i] /= ddj
         end
     end
     A

src/julia-syntax.scm

@@ -1647,6 +1647,10 @@
             (cons (car e)
                   (map expand-forms (cdr e)))))))
 
+;; If true, this will warn on all `for` loop variables that overwrite outer variables.
+;; If false, this will try to warn only for uses of the last value after the loop.
+(define *warn-all-loop-vars* #f)
+
 (define (expand-for while lhs X body)
   ;; (for (= lhs X) body)
   (let ((coll  (make-ssavalue))
@@ -1661,6 +1665,9 @@
                   (block
                    ;; NOTE: enable this to force loop-local var
                    #;,@(map (lambda (v) `(local ,v)) (lhs-vars lhs))
+                   ,@(if (or *depwarn* *deperror*)
+                         (map (lambda (v) `(warn-if-existing ,v)) (lhs-vars lhs))
+                         '())
                    ,(lower-tuple-assignment (list lhs state)
                                             `(call (top next) ,coll ,state))
                    ,body))))))))
@@ -2491,7 +2498,7 @@
         ((eq? (car e) 'break-block) (unbound-vars (caddr e) bound tab))
         ((eq? (car e) 'with-static-parameters) (unbound-vars (cadr e) bound tab))
         (else (for-each (lambda (x) (unbound-vars x bound tab))
-                            (cdr e))
+                        (cdr e))
               tab)))
 
 ;; local variable identification and renaming, derived from:
@@ -2507,6 +2514,7 @@
         ((eq? (car e) 'local) '(null)) ;; remove local decls
         ((eq? (car e) 'local-def) '(null)) ;; remove local decls
         ((eq? (car e) 'implicit-global) '(null)) ;; remove implicit-global decls
+        ((eq? (car e) 'warn-if-existing) '(null))
         ((eq? (car e) 'lambda)
          (let* ((lv (lam:vars e))
                 (env (append lv env))
@@ -2547,6 +2555,9 @@
                                vars))))
                 (need-rename (need-rename? vars))
                 (need-rename-def (need-rename? vars-def))
+                (deprecated-loop-vars
+                 (filter (lambda (v) (memq v env))
+                         (delete-duplicates (find-decls 'warn-if-existing blok))))
                 ;; new gensym names for conflicting variables
                 (renamed (map named-gensy need-rename))
                 (renamed-def (map named-gensy need-rename-def))
@@ -2576,12 +2587,21 @@
                (if lam ;; update in-place the list of local variables in lam
                    (set-car! (cddr lam)
                              (append! (caddr lam) real-new-vars real-new-vars-def)))
-               (insert-after-meta ;; return the new, expanded scope-block
-                (if (and (pair? body) (eq? (car body) 'block))
-                    body
-                    `(block ,body))
-                (append! (map (lambda (v) `(local ,v)) real-new-vars)
-                         (map (lambda (v) `(local-def ,v)) real-new-vars-def)))))
+               (let* ((warnings (map (lambda (v) `(warn-loop-var ,v)) deprecated-loop-vars))
+                      (body (if *warn-all-loop-vars*
+                                body
+                                (if (and (pair? body) (eq? (car body) 'block))
+                                    (append body warnings)
+                                    `(block ,body ,@warnings)))))
+                 (insert-after-meta ;; return the new, expanded scope-block
+                  (if (and (pair? body) (eq? (car body) 'block))
+                      body
+                      `(block ,body))
+                  (append! (map (lambda (v) `(local ,v)) real-new-vars)
+                           (map (lambda (v) `(local-def ,v)) real-new-vars-def)
+                           (if *warn-all-loop-vars*
+                               (map (lambda (v) `(warn-loop-var ,v)) deprecated-loop-vars)
+                               '()))))))
         ((eq? (car e) 'module)
          (error "module expression not at top level"))
         ((eq? (car e) 'break-block)
@@ -3035,7 +3055,7 @@ f(x) = yt(x)
        ((atom? e) e)
        (else
         (case (car e)
-          ((quote top core globalref outerref line break inert module toplevel null meta) e)
+          ((quote top core globalref outerref line break inert module toplevel null meta warn-loop-var) e)
           ((=)
            (let ((var (cadr e))
                  (rhs (cl-convert (caddr e) fname lam namemap toplevel interp)))
@@ -3282,6 +3302,11 @@ f(x) = yt(x)
         (else (for-each linearize (cdr e))))
   e)
 
+(define (deprecation-message msg)
+  (if *deperror*
+      (error msg)
+      (io.write *stderr* msg)))
+
 ;; this pass behaves like an interpreter on the given code.
 ;; to perform stateful operations, it calls `emit` to record that something
 ;; needs to be done. in value position, it returns an expression computing
@@ -3294,6 +3319,7 @@ f(x) = yt(x)
         (first-line #t)
         (current-loc #f)
         (rett #f)
+        (deprecated-loop-vars (table))
         (arg-map #f)          ;; map arguments to new names if they are assigned
         (label-counter 0)     ;; counter for generating label addresses
         (label-map (table))   ;; maps label names to generated addresses
@@ -3387,6 +3413,11 @@ f(x) = yt(x)
                                     (eq? (cadr e) '_))))
                 (syntax-deprecation #f (string "_ as an rvalue" (linenode-string current-loc))
                                     ""))
+            (if (and (not *warn-all-loop-vars*) (has? deprecated-loop-vars e))
+                (begin (deprecation-message
+                        (string "Use of final value of loop variable \"" e "\"" (linenode-string current-loc) " "
+                                "is deprecated. In the future the variable will be local to the loop instead." #\newline))
+                       (del! deprecated-loop-vars e)))
             (cond (tail  (emit-return e1))
                   (value e1)
                   ((or (eq? e1 'true) (eq? e1 'false)) #f)
@@ -3416,6 +3447,8 @@ f(x) = yt(x)
                     (lhs (if (and arg-map (symbol? lhs))
                              (get arg-map lhs lhs)
                              lhs)))
+               (if (and (not *warn-all-loop-vars*) (has? deprecated-loop-vars lhs))
+                   (del! deprecated-loop-vars lhs))
                (if value
                    (let ((rr (if (or (atom? rhs) (ssavalue? rhs) (eq? (car rhs) 'null))
                                  rhs (make-ssavalue))))
@@ -3602,6 +3635,14 @@ f(x) = yt(x)
             ((implicit-global) #f)
             ((const) (emit e))
             ((isdefined) (if tail (emit-return e) e))
+            ((warn-loop-var)
+             (if *warn-all-loop-vars*
+                 (deprecation-message
+                  (string "Loop variable \"" (cadr e) "\"" (linenode-string current-loc) " "
+                          "overwrites a variable in an enclosing scope. "
+                          "In the future the variable will be local to the loop instead." #\newline))
+                 (put! deprecated-loop-vars (cadr e) #t))
+             '(null))
 
             ;; top level expressions returning values
             ((abstract_type bits_type composite_type thunk toplevel module)

test/read.jl

@@ -176,13 +176,14 @@ for (name, f) in l
     old_text = text
     cleanup()
 
-    for text in [
+    for text_ in [
         old_text,
         String(Char['A' + i % 52 for i in 1:(div(Base.SZ_UNBUFFERED_IO,2))]),
         String(Char['A' + i % 52 for i in 1:(    Base.SZ_UNBUFFERED_IO -1)]),
         String(Char['A' + i % 52 for i in 1:(    Base.SZ_UNBUFFERED_IO   )]),
         String(Char['A' + i % 52 for i in 1:(    Base.SZ_UNBUFFERED_IO +1)])
     ]
+        text = text_
         write(filename, text)
 
         verbose && println("$name readstring...")

test/sparse/sparse.jl

@@ -1095,27 +1095,27 @@ end
     N=2^3
     Irand = randperm(M)
     Jrand = randperm(N)
-    I = sort([Irand; Irand; Irand])
+    II = sort([Irand; Irand; Irand])
     J = [Jrand; Jrand]
 
     SA = [sprand(M, N, d) for d in [1., 0.1, 0.01, 0.001, 0.0001, 0.]]
     for S in SA
         res = Any[1,2,3]
         for searchtype in [0, 1, 2]
-            res[searchtype+1] = test_getindex_algs(S, I, J, searchtype)
+            res[searchtype+1] = test_getindex_algs(S, II, J, searchtype)
         end
 
         @test res[1] == res[2] == res[3]
     end
 
     M = 2^14
     N=2^4
-    I = randperm(M)
+    II = randperm(M)
     J = randperm(N)
     Jsorted = sort(J)
 
     SA = [sprand(M, N, d) for d in [1., 0.1, 0.01, 0.001, 0.0001, 0.]]
-    IA = [I[1:round(Int,n)] for n in [M, M*0.1, M*0.01, M*0.001, M*0.0001, 0.]]
+    IA = [II[1:round(Int,n)] for n in [M, M*0.1, M*0.01, M*0.001, M*0.0001, 0.]]
     debug = false
     if debug
         @printf("         |         |         |        times        |        memory       |\n")