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
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 committed Nov 8, 2024
1 parent 8593792 commit 3c0c5a2
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 @@ -985,6 +985,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 3c0c5a2

Please sign in to comment.