Skip to content

Commit 9a437a5

Browse files
committed
const lowering: respect scope, prohibit non-const const assignment
Modifies the handling of the 3-argument "lowered const" so `const x = y` participates in scope resolution again (#56613); we treat (const x y) as an assignment form, producing the "unsupported `const` declaration on local variable" error if the usual scope rules would not make x global. At the top level, these should succeed: const x = 1 begin const x = 1 end let const global x = 1 end Each of the following should fail: # ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 let const x = 1 end # ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 function f() const x = 1 end This also amkes the use of `const global` inside a function body produce the original, specific error message rather than a generic "World age increment not at top level" error (#57334): # ERROR: syntax: `global const` declaration not allowed inside function around REPL[1]:2 function f() global const x = 1 end Issue #57334 revealed we silently accept `const` expressions writing to variables. These now fail with a new error message: a = [1, 2] # ERROR: syntax: cannot declare "a[1]" `const` around REPL[1]:2 const a[1] = 3 The bulk of these changes track const-ness through the functions called by expand-assignment, since each of the three types of LHS is affect differently: - ordinary variables become `const`, producing error if not global - `X{T} = ...` forces X to be `const`, rather than the defaulting to being implicitly const only if in global scope. - `f() = ...` ignores `const`.
1 parent 6043569 commit 9a437a5

File tree

2 files changed

+120
-76
lines changed

2 files changed

+120
-76
lines changed

src/julia-syntax.scm

Lines changed: 111 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,7 +1429,7 @@
14291429
(else
14301430
(error "invalid \"try\" form")))))
14311431

1432-
(define (expand-unionall-def name type-ex (allow-local #t))
1432+
(define (expand-unionall-def name type-ex (const? #t))
14331433
(if (and (pair? name)
14341434
(eq? (car name) 'curly))
14351435
(let ((name (cadr name))
@@ -1440,7 +1440,7 @@
14401440
(expand-forms
14411441
`(block
14421442
(= ,rr (where ,type-ex ,@params))
1443-
(,(if allow-local 'assign-const-if-global 'const) ,name ,rr)
1443+
(,(if const? 'const 'assign-const-if-global) ,name ,rr)
14441444
(latestworld-if-toplevel)
14451445
,rr)))
14461446
(expand-forms
@@ -1450,38 +1450,24 @@
14501450
(filter (lambda (x) (not (underscore-symbol? x))) syms))
14511451

14521452
;; Expand `[global] const a::T = val`
1453-
(define (expand-const-decl e (mustassgn #f))
1454-
(if (length= e 3) e
1455-
(let ((arg (cadr e)))
1456-
(if (atom? arg)
1457-
(if mustassgn
1458-
(error "expected assignment after \"const\"")
1459-
e)
1460-
(case (car arg)
1461-
((global)
1462-
(expand-const-decl `(const ,(cadr arg)) #t))
1463-
((=)
1464-
(cond
1465-
;; `const f() = ...` - The `const` here is inoperative, but the syntax happened to work in earlier versions, so simply strip `const`.
1466-
;; TODO: Consider whether to keep this in 2.0.
1467-
((eventually-call? (cadr arg))
1468-
(expand-forms arg))
1469-
((and (pair? (cadr arg)) (eq? (caadr arg) 'curly))
1470-
(expand-unionall-def (cadr arg) (caddr arg)))
1471-
((and (pair? (cadr arg)) (eq? (caadr arg) 'tuple) (not (has-parameters? (cdr (cadr arg)))))
1472-
;; We need this case because `(f(), g()) = (1, 2)` goes through here, which cannot go via the `local` lowering below,
1473-
;; because the symbols come out wrong. Sigh... So much effort for such a syntax corner case.
1474-
(expand-tuple-destruct (cdr (cadr arg)) (caddr arg) (lambda (assgn) `(,(car e) ,assgn))))
1475-
(else
1476-
(let ((rr (make-ssavalue)))
1477-
(expand-forms `(block
1478-
(= ,rr ,(caddr arg))
1479-
(scope-block (block (hardscope)
1480-
(local (= ,(cadr arg) ,rr))
1481-
,.(map (lambda (v) `(,(car e) (globalref (thismodule) ,v) ,v)) (filter-not-underscore (lhs-vars (cadr arg))))
1482-
(latestworld)
1483-
,rr))))))))
1484-
(else (error "expected assignment after \"const\"")))))))
1453+
(define (expand-const-decl e)
1454+
(define (check-assignment asgn)
1455+
(unless (and (pair? asgn) (eq? (car asgn) '=))
1456+
;; (const (global x)) is possible due to a parser quirk
1457+
(error "expected assignment after \"const\"")))
1458+
(if (length= e 3)
1459+
`(const ,(cadr e) ,(expand-forms (caddr e)))
1460+
(let ((arg (cadr e)))
1461+
(case (car arg)
1462+
((global) (let ((asgn (cadr arg)))
1463+
(check-assignment asgn)
1464+
`(block
1465+
,.(map (lambda (v) `(global ,v))
1466+
(lhs-bound-names (cadr asgn)))
1467+
,(expand-assignment asgn #t))))
1468+
((=) (check-assignment arg)
1469+
(expand-assignment arg #t))
1470+
(else (error "expected assignment after \"const\""))))))
14851471

14861472
(define (expand-atomic-decl e)
14871473
(error "unimplemented or unsupported atomic declaration"))
@@ -1538,35 +1524,56 @@
15381524
(eq? (car (cadr lhs)) 'call)))))
15391525
(define (assignment-to-function lhs e) ;; convert '= expr to 'function expr
15401526
(cons 'function (cdr e)))
1527+
(define (maybe-wrap-const x)
1528+
(if const? `(const ,x) x))
15411529
(cond
15421530
((function-lhs? lhs)
1531+
;; `const f() = ...` - The `const` here is inoperative, but the syntax
1532+
;; happened to work in earlier versions, so simply strip `const`.
15431533
(expand-forms (assignment-to-function lhs e)))
15441534
((and (pair? lhs)
15451535
(eq? (car lhs) 'curly))
1546-
(expand-unionall-def (cadr e) (caddr e)))
1536+
(expand-unionall-def (cadr e) (caddr e) const?))
15471537
((assignment? (caddr e))
15481538
;; chain of assignments - convert a=b=c to `b=c; a=c`
15491539
(let loop ((lhss (list lhs))
15501540
(rhs (caddr e)))
15511541
(if (and (assignment? rhs) (not (function-lhs? (cadr rhs))))
15521542
(loop (cons (cadr rhs) lhss) (caddr rhs))
1553-
(let ((rr (if (symbol-like? rhs) rhs (make-ssavalue))))
1543+
(let* ((rr (if (symbol-like? rhs) rhs (make-ssavalue)))
1544+
(lhss (reverse lhss))
1545+
(lhs0 (car lhss))
1546+
(lhss (cdr lhss))
1547+
(lhss (reverse lhss)))
15541548
(expand-forms
15551549
`(block ,.(if (eq? rr rhs) '() `((= ,rr ,(if (assignment? rhs)
15561550
(assignment-to-function (cadr rhs) rhs)
15571551
rhs))))
1558-
,@(map (lambda (l) `(= ,l ,rr))
1559-
lhss)
1552+
,@(map (lambda (l) `(= ,l ,rr)) lhss)
1553+
;; In const x = y = z, only x becomes const
1554+
,(maybe-wrap-const `(= ,lhs0 ,rr))
15601555
(unnecessary ,rr)))))))
15611556
((or (and (symbol-like? lhs) (valid-name? lhs))
15621557
(globalref? lhs))
1563-
(sink-assignment lhs (expand-forms (caddr e))))
1558+
;; TODO: We currently call (latestworld) after every (const _ _), but this
1559+
;; may need to be moved elsewhere if we want to avoid making one const
1560+
;; visible before side effects have been performed (#57484)
1561+
(if const?
1562+
(let ((rr (make-ssavalue)))
1563+
`(block
1564+
,(sink-assignment rr (expand-forms (caddr e)))
1565+
(const ,lhs ,rr)
1566+
(latestworld)
1567+
(unnecessary ,rr)))
1568+
(sink-assignment lhs (expand-forms (caddr e)))))
15641569
((atom? lhs)
15651570
(error (string "invalid assignment location \"" (deparse lhs) "\"")))
15661571
(else
15671572
(case (car lhs)
15681573
((|.|)
15691574
;; a.b =
1575+
(when const?
1576+
(error (string "cannot declare \"" (deparse lhs) "\" `const`")))
15701577
(let* ((a (cadr lhs))
15711578
(b (caddr lhs))
15721579
(rhs (caddr e)))
@@ -1588,15 +1595,17 @@
15881595
(x (caddr e)))
15891596
(if (has-parameters? lhss)
15901597
;; property destructuring
1591-
(expand-property-destruct lhss x)
1598+
(expand-property-destruct lhss x maybe-wrap-const)
15921599
;; multiple assignment
1593-
(expand-tuple-destruct lhss x))))
1600+
(expand-tuple-destruct lhss x maybe-wrap-const))))
15941601
((typed_hcat)
15951602
(error "invalid spacing in left side of indexed assignment"))
15961603
((typed_vcat typed_ncat)
15971604
(error "unexpected \";\" in left side of indexed assignment"))
15981605
((ref)
15991606
;; (= (ref a . idxs) rhs)
1607+
(when const?
1608+
(error (string "cannot declare \"" (deparse lhs) "\" `const`")))
16001609
(let ((a (cadr lhs))
16011610
(idxs (cddr lhs))
16021611
(rhs (caddr e)))
@@ -1626,10 +1635,24 @@
16261635
(T (caddr lhs))
16271636
(rhs (caddr e)))
16281637
(let ((e (remove-argument-side-effects x)))
1629-
(expand-forms
1630-
`(block ,@(cdr e)
1631-
(decl ,(car e) ,T)
1632-
(= ,(car e) ,rhs))))))
1638+
(if const?
1639+
;; This could go through convert-assignment in the closure
1640+
;; conversion pass, but since constants don't have declared types
1641+
;; the way other variables do, we insert convert() here.
1642+
(expand-forms
1643+
;; TODO: This behaviour (`const _:T = ...` does not call convert,
1644+
;; but still evaluates RHS) should be documented.
1645+
`(const ,(car e) ,(if (underscore-symbol? (car e))
1646+
rhs
1647+
(convert-for-type-decl rhs T #t #f))))
1648+
(expand-forms
1649+
`(block ,@(cdr e)
1650+
;; TODO: When x is a complex expression, this acts as a
1651+
;; typeassert rather than a declaration.
1652+
,.(if (underscore-symbol? (car e))
1653+
'() ; Assignment to _ will ultimately be discarded---don't declare anything
1654+
`((decl ,(car e) ,T)))
1655+
,(maybe-wrap-const `(= ,(car e) ,rhs))))))))
16331656
((vcat ncat)
16341657
;; (= (vcat . args) rhs)
16351658
(error "use \"(a, b) = ...\" to assign multiple values"))
@@ -2371,7 +2394,7 @@
23712394
(gensy))
23722395
(else (make-ssavalue))))
23732396

2374-
(define (expand-property-destruct lhs x)
2397+
(define (expand-property-destruct lhs x (wrap identity))
23752398
(if (not (length= lhs 1))
23762399
(error (string "invalid assignment location \"" (deparse `(tuple ,lhs)) "\"")))
23772400
(let* ((lhss (cdar lhs))
@@ -2386,7 +2409,7 @@
23862409
(cadr field))
23872410
(else
23882411
(error (string "invalid assignment location \"" (deparse `(tuple ,lhs)) "\""))))))
2389-
(expand-forms `(= ,field (call (top getproperty) ,xx (quote ,prop))))))
2412+
(expand-forms (wrap `(= ,field (call (top getproperty) ,xx (quote ,prop)))))))
23902413
lhss)
23912414
(unnecessary ,xx))))
23922415

@@ -2407,7 +2430,6 @@
24072430
(if (null? lhss)
24082431
'()
24092432
(let* ((lhs (car lhss))
2410-
(wrapfirst (lambda (x i) (if (= i 1) (wrap x) x)))
24112433
(lhs- (cond ((or (symbol? lhs) (ssavalue? lhs))
24122434
lhs)
24132435
((vararg? lhs)
@@ -2419,7 +2441,10 @@
24192441
(make-ssavalue))))))
24202442
;; can't use ssavalues if it's a function definition
24212443
((eventually-call? lhs) (gensy))
2422-
(else (make-ssavalue)))))
2444+
(else (make-ssavalue))))
2445+
;; If we use an intermediary lhs, don't wrap `const`.
2446+
(wrap-subassign (if (eq? lhs lhs-) wrap identity))
2447+
(wrapfirst (lambda (x i) (if (= i 1) (wrap-subassign x) x))))
24232448
(if (and (vararg? lhs) (any vararg? (cdr lhss)))
24242449
(error "multiple \"...\" on lhs of assignment"))
24252450
(if (not (eq? lhs lhs-))
@@ -2431,7 +2456,7 @@
24312456
(if (underscore-symbol? (cadr lhs-))
24322457
'()
24332458
(list (expand-forms
2434-
(wrap `(= ,(cadr lhs-) (call (top rest) ,xx ,@(if (eq? i 1) '() `(,st))))))))
2459+
(wrap-subassign `(= ,(cadr lhs-) (call (top rest) ,xx ,@(if (eq? i 1) '() `(,st))))))))
24352460
(let ((tail (if (eventually-call? lhs) (gensy) (make-ssavalue))))
24362461
(cons (expand-forms
24372462
(lower-tuple-assignment
@@ -2978,6 +3003,16 @@
29783003
(define (lhs-vars e)
29793004
(map decl-var (lhs-decls e)))
29803005

3006+
;; Return all the names that will be bound by the assignment LHS, including
3007+
;; curlies and calls.
3008+
(define (lhs-bound-names e)
3009+
(cond ((underscore-symbol? e) '())
3010+
((atom? e) (list e))
3011+
((and (pair? e) (memq (car e) '(call curly where |::|)))
3012+
(lhs-bound-names (cadr e)))
3013+
((and (pair? e) (memq (car e) '(tuple parameters)))
3014+
(apply append (map lhs-bound-names (cdr e))))))
3015+
29813016
(define (all-decl-vars e) ;; map decl-var over every level of an assignment LHS
29823017
(cond ((eventually-call? e) e)
29833018
((decl? e) (decl-var e))
@@ -3004,7 +3039,7 @@
30043039
;; like v = val, except that if `v` turns out global(either
30053040
;; implicitly or by explicit `global`), it gains an implicit `const`
30063041
(set! vars (cons (cadr e) vars)))
3007-
((=)
3042+
((= const)
30083043
(let ((v (decl-var (cadr e))))
30093044
(find-assigned-vars- (caddr e))
30103045
(if (or (ssavalue? v) (globalref? v) (underscore-symbol? v))
@@ -3130,14 +3165,16 @@
31303165
((eq? (car e) 'global)
31313166
(check-valid-name (cadr e))
31323167
e)
3168+
31333169
((eq? (car e) 'assign-const-if-global)
31343170
(if (eq? (var-kind (cadr e) scope) 'local)
31353171
(if (length= e 2) (null) `(= ,@(cdr e)))
3136-
`(const ,@(cdr e))))
3172+
(resolve-scopes- `(const ,@(cdr e)) scope sp loc)))
31373173
((eq? (car e) 'global-if-global)
31383174
(if (eq? (var-kind (cadr e) scope) 'local)
31393175
'(null)
31403176
`(global ,@(cdr e))))
3177+
31413178
((memq (car e) '(local local-def))
31423179
(check-valid-name (cadr e))
31433180
;; remove local decls
@@ -3290,7 +3327,7 @@
32903327
,(resolve-scopes- (caddr e) scope)
32913328
,(resolve-scopes- (cadddr e) scope (method-expr-static-parameters e))))
32923329
(else
3293-
(if (and (eq? (car e) '=) (symbol? (cadr e))
3330+
(if (and (memq (car e) '(= const)) (symbol? (cadr e))
32943331
scope (null? (lam:args (scope:lam scope)))
32953332
(warn-var?! (cadr e) scope)
32963333
(= *scopewarn-opt* 1))
@@ -3410,7 +3447,7 @@
34103447
((local-def) ;; a local that we know has an assignment that dominates all usages
34113448
(let ((vi (get tab (cadr e) #f)))
34123449
(vinfo:set-never-undef! vi #t)))
3413-
((=)
3450+
((= const)
34143451
(let ((vi (and (symbol? (cadr e)) (get tab (cadr e) #f))))
34153452
(if vi ; if local or captured
34163453
(begin (if (vinfo:asgn vi)
@@ -4027,7 +4064,10 @@ f(x) = yt(x)
40274064
'(null)
40284065
`(newvar ,(cadr e))))))
40294066
((const)
4030-
(put! globals (binding-to-globalref (cadr e)) #f)
4067+
;; Check we've expanded surface `const` (1 argument form)
4068+
(assert (and (length= e 3)))
4069+
(when (globalref? (cadr e))
4070+
(put! globals (cadr e) #f))
40314071
e)
40324072
((atomic) e)
40334073
((isdefined) ;; convert isdefined expr to function for closure converted variables
@@ -4379,7 +4419,6 @@ f(x) = yt(x)
43794419
(first-line #t)
43804420
(current-loc #f)
43814421
(rett #f)
4382-
(global-const-error #f)
43834422
(vinfo-table (vinfo-to-table (car (lam:vinfo lam))))
43844423
(arg-map #f) ;; map arguments to new names if they are assigned
43854424
(label-counter 0) ;; counter for generating label addresses
@@ -4592,18 +4631,19 @@ f(x) = yt(x)
45924631
(cdr cnd)
45934632
(list cnd))))))
45944633
tests))
4595-
(define (emit-assignment-or-setglobal lhs rhs)
4596-
(if (globalref? lhs)
4634+
(define (emit-assignment-or-setglobal lhs rhs (op '=))
4635+
;; (const (globalref _ _) _) does not use setglobal!
4636+
(if (and (globalref? lhs) (eq? op '=))
45974637
(emit `(call (top setglobal!) ,(cadr lhs) (inert ,(caddr lhs)) ,rhs))
4598-
(emit `(= ,lhs ,rhs))))
4599-
(define (emit-assignment lhs rhs)
4638+
(emit `(,op ,lhs ,rhs))))
4639+
(define (emit-assignment lhs rhs (op '=))
46004640
(if rhs
46014641
(if (valid-ir-rvalue? lhs rhs)
4602-
(emit-assignment-or-setglobal lhs rhs)
4642+
(emit-assignment-or-setglobal lhs rhs op)
46034643
(let ((rr (make-ssavalue)))
46044644
(emit `(= ,rr ,rhs))
4605-
(emit-assignment-or-setglobal lhs rr)))
4606-
(emit-assignment-or-setglobal lhs `(null))) ; in unreachable code (such as after return), still emit the assignment so that the structure of those uses is preserved
4645+
(emit-assignment-or-setglobal lhs rr op)))
4646+
(emit-assignment-or-setglobal lhs `(null) op)) ; in unreachable code (such as after return), still emit the assignment so that the structure of those uses is preserved
46074647
#f)
46084648
;; the interpreter loop. `break-labels` keeps track of the labels to jump to
46094649
;; for all currently closing break-blocks.
@@ -4669,7 +4709,12 @@ f(x) = yt(x)
46694709
(cond (tail (emit-return tail callex))
46704710
(value callex)
46714711
(else (emit callex)))))
4672-
((=)
4712+
((= const)
4713+
(when (eq? (car e) 'const)
4714+
(when (local-in? (cadr e) lam)
4715+
(error (string "unsupported `const` declaration on local variable" (format-loc current-loc))))
4716+
(when (pair? (cadr lam))
4717+
(error (string "`global const` declaration not allowed inside function" (format-loc current-loc)))))
46734718
(let ((lhs (cadr e)))
46744719
(if (and (symbol? lhs) (underscore-symbol? lhs))
46754720
(compile (caddr e) break-labels value tail)
@@ -4682,10 +4727,10 @@ f(x) = yt(x)
46824727
rhs (make-ssavalue))))
46834728
(if (not (eq? rr rhs))
46844729
(emit `(= ,rr ,rhs)))
4685-
(emit-assignment-or-setglobal lhs rr)
4730+
(emit-assignment-or-setglobal lhs rr (car e))
46864731
(if tail (emit-return tail rr))
46874732
rr)
4688-
(emit-assignment lhs rhs))))))
4733+
(emit-assignment lhs rhs (car e)))))))
46894734
((block)
46904735
(let* ((last-fname filename)
46914736
(fnm (first-non-meta e))
@@ -4928,14 +4973,6 @@ f(x) = yt(x)
49284973
((moved-local)
49294974
(set-car! (lam:vinfo lam) (append (car (lam:vinfo lam)) `((,(cadr e) Any 2))))
49304975
#f)
4931-
((const)
4932-
(if (local-in? (cadr e) lam)
4933-
(error (string "unsupported `const` declaration on local variable" (format-loc current-loc)))
4934-
(if (pair? (cadr lam))
4935-
;; delay this error to allow "misplaced struct" errors to happen first
4936-
(if (not global-const-error)
4937-
(set! global-const-error current-loc))
4938-
(emit e))))
49394976
((atomic) (error "misplaced atomic declaration"))
49404977
((isdefined throw_undef_if_not) (if tail (emit-return tail e) e))
49414978
((boundscheck) (if tail (emit-return tail e) e))
@@ -5066,8 +5103,6 @@ f(x) = yt(x)
50665103
(let ((pexc (pop-exc-expr src-catch-tokens target-catch-tokens)))
50675104
(if pexc (set-cdr! point (cons pexc (cdr point)))))))))
50685105
handler-goto-fixups)
5069-
(if global-const-error
5070-
(error (string "`global const` declaration not allowed inside function" (format-loc global-const-error))))
50715106
(let* ((stmts (reverse! code))
50725107
(di (definitely-initialized-vars stmts vi))
50735108
(body (cons 'block (filter (lambda (e)

0 commit comments

Comments
 (0)