From 3c0c5a2e8b2248bdb7f7ba80b40aab3d63df5b28 Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@juliahub.com>
Date: Fri, 8 Nov 2024 00:55:03 +0000
Subject: [PATCH] Undo the decision to publish incomplete types to the binding
 table

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.
---
 base/boot.jl         |  8 ++++++++
 src/julia-syntax.scm | 21 ++++++++++++++++-----
 src/utils.scm        |  7 +++++++
 test/core.jl         |  2 +-
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/base/boot.jl b/base/boot.jl
index 5d40191ecab21..50b46758efe2e 100644
--- a/base/boot.jl
+++ b/base/boot.jl
@@ -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)
diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
index b48cb48bf0b79..7acc8a1954bc5 100644
--- a/src/julia-syntax.scm
+++ b/src/julia-syntax.scm
@@ -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)
@@ -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
diff --git a/src/utils.scm b/src/utils.scm
index 97464b9a14e5a..79e3a280b9886 100644
--- a/src/utils.scm
+++ b/src/utils.scm
@@ -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)
diff --git a/test/core.jl b/test/core.jl
index 4b5a674ba44b3..1b36db466ce19 100644
--- a/test/core.jl
+++ b/test/core.jl
@@ -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