Skip to content

Commit

Permalink
Undo the decision to publish incomplete types to the binding table (#…
Browse files Browse the repository at this point in the history
…56497)

This effectively reverts #36121 and replaces it with #36111, which was
the originally proposed alternative to fix #36104. To recap, the
question is what should happen for
```
module Foo
    struct F
        v::Foo.F
    end
end
```
i.e. where the type reference tries to refer to the newly defined type
via its global path. In #36121 we adjusted things so that we first
assign the type to its global binding and then evaluate the field type
(leaving the type in an incomplete state in the meantime). The primary
reason that this choice was that we would have to deal with incomplete
types assigned to global bindings anyway if we ever did #32658. However,
I think this was the wrong choice. There is a difference between
allowing incomplete types and semantically forcing incomplete types to
be globally observable every time a new type is defined.

The situation was a little different four years ago, but with more
extensive threading (which can observe the incompletely constructed
type) and the upcoming completion of bindings partition, the situation
is different. For bindings partition in particular, this would require
two invalidations on re-definition, one to the new incomplete type and
then back to the complete type. I don't think this is worth it, for the
(somewhat niche and possibly-should-be- deprecated-future) case of
refering to incompletely defined types by their global names.

So let's instead try the hack in #36111, which does a frontend rewrite
of the global path. This should be sufficient to at least address the
obvious cases.
  • Loading branch information
Keno authored Nov 9, 2024
1 parent 4cbeea5 commit 435516d
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 6 deletions.
8 changes: 8 additions & 0 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,14 @@ Unsigned(x::Union{Float16, Float32, Float64, Bool}) = UInt(x)
Integer(x::Integer) = x
Integer(x::Union{Float16, Float32, Float64}) = Int(x)

# During definition of struct type `B`, if an `A.B` expression refers to
# the eventual global name of the struct, then return the partially-initialized
# type object.
# TODO: remove. This is a shim for backwards compatibility.
function struct_name_shim(@nospecialize(x), name::Symbol, mod::Module, @nospecialize(t))
return x === mod ? t : getfield(x, name)
end

# Binding for the julia parser, called as
#
# Core._parse(text, filename, lineno, offset, options)
Expand Down
21 changes: 16 additions & 5 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,19 @@
(ctors-min-initialized (car expr))
(ctors-min-initialized (cdr expr)))))

(define (insert-struct-shim field-types name)
(map (lambda (x)
(expr-replace (lambda (y)
(and (length= y 3) (eq? (car y) '|.|)
(or (equal? (caddr y) `(quote ,name))
(equal? (caddr y) `(inert ,name)))))
x
(lambda (y)
`(call (core struct_name_shim)
,(cadr y) ,(caddr y)
(thismodule) ,name))))
field-types))

(define (struct-def-expr- name params bounds super fields0 mut)
(receive
(fields defs) (separate eventually-decl? fields0)
Expand Down Expand Up @@ -1022,11 +1035,9 @@
prev
params)
(quote parameters))))
'()))
;; otherwise do an assignment to trigger an error
(const (globalref (thismodule) ,name) ,name)))
(const (globalref (thismodule) ,name) ,name))
(call (core _typebody!) ,name (call (core svec) ,@field-types))
'())))))
(call (core _typebody!) ,name (call (core svec) ,@(insert-struct-shim field-types name)))
(const (globalref (thismodule) ,name) ,name)
(null)))
;; "inner" constructors
(scope-block
Expand Down
7 changes: 7 additions & 0 deletions src/utils.scm
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@
(any (lambda (y) (expr-contains-p p y filt))
(cdr expr))))))

(define (expr-replace p expr repl)
(cond ((p expr) (repl expr))
((and (pair? expr) (not (quoted? expr)))
(cons (car expr)
(map (lambda (x) (expr-replace p x repl)) (cdr expr))))
(else expr)))

;; find all subexprs satisfying `p`, applying `key` to each one
(define (expr-find-all p expr key (filt (lambda (x) #t)))
(if (filt expr)
Expand Down
2 changes: 1 addition & 1 deletion test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7669,7 +7669,7 @@ end
end
@test fieldtypes(M36104.T36104) == (Vector{M36104.T36104},)
@test_throws ErrorException("expected") @eval(struct X36104; x::error("expected"); end)
@test @isdefined(X36104)
@test !@isdefined(X36104)
struct X36104; x::Int; end
@test fieldtypes(X36104) == (Int,)
primitive type P36104 8 end
Expand Down

0 comments on commit 435516d

Please sign in to comment.