just use binding table

d-netto · d-netto · commit e93d2bc544a1 · 2024-09-03T11:26:17.000-03:00
diff --git a/src/ast.c b/src/ast.c
@@ -8,8 +8,6 @@
 #include <stdio.h>
 #include <string.h>
 
-#include "support/strhash.h"
-
 #ifdef _OS_WINDOWS_
 #include <malloc.h>
 #endif
@@ -218,53 +216,15 @@ static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint
         decode_restriction_value(pku) : jl_atomic_load_relaxed(&b->value)) != NULL ? fl_ctx->T : fl_ctx->F;
 }
 
-static htable_t *rebuild_counter_table(jl_module_t *m) JL_NOTSAFEPOINT
-{
-    htable_t *counter_table = m->counter_table = htable_new((htable_t *)malloc_s(sizeof(htable_t)), 0);
-    jl_svec_t *t = jl_atomic_load_relaxed(&m->bindings);
-    for (size_t i = 0; i < jl_svec_len(t); i++) {
-        jl_binding_t *b = (jl_binding_t*)jl_svecref(t, i);
-        if ((void*)b == jl_nothing) {
-            continue;
-        }
-        char *globalref_name = jl_symbol_name(b->globalref->name);
-        if (is_canonicalized_anonfn_typename(globalref_name)) {
-            int should_free = 1;
-            // copy globalref_name into the buffer until we hit a `#` character
-            char *delim = strchr(&globalref_name[1], '#');
-            assert(delim != NULL);
-            size_t len = delim - globalref_name - 1;
-            assert(len > 0);
-            char *enclosing_function_name = (char*)calloc_s(len + 1);
-            memcpy(enclosing_function_name, &globalref_name[1], len);
-            // check if the enclosing function name is already in the counter table
-            if (strhash_get(counter_table, enclosing_function_name) == HT_NOTFOUND) {
-                strhash_put(counter_table, enclosing_function_name, (void*)((uintptr_t)HT_NOTFOUND + 1));
-                should_free = 0;
-            }
-            char *pint = strrchr(globalref_name, '#');
-            assert(pint != NULL);
-            int counter = atoi(pint + 1);
-            int max_seen_so_far = ((uint32_t)(uintptr_t)strhash_get(counter_table, enclosing_function_name) - (uintptr_t)HT_NOTFOUND - 1);
-            if (counter >= max_seen_so_far) {
-                strhash_put(counter_table, enclosing_function_name, (void*)((uintptr_t)counter + 1 + (uintptr_t)HT_NOTFOUND + 1));
-            }
-            if (should_free) {
-                free(enclosing_function_name);
-            }
-        }
-    }
-    return counter_table;
-}
-
-// used to generate a unique suffix for a given symbol (e.g. variable or type name)
+// Used to generate a unique suffix for a given symbol (e.g. variable or type name)
 // first argument contains a stack of method definitions seen so far by `closure-convert` in flisp.
 // if the top of the stack is non-NIL, we use it to augment the suffix so that it becomes
-// of the form $top_level_method_name##$counter, where counter is stored in a per-module
-// side table indexed by top-level method name.
-// this ensures that precompile statements are a bit more stable across different versions
+// of the form $top_level_method_name##$counter, where `counter` is the smallest integer
+// such that the resulting name is not already defined in the current module's bindings.
+// If the top of the stack is NIL, we simply return the current module's counter.
+// This ensures that precompile statements are a bit more stable across different versions
 // of a codebase. see #53719
-static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT
+static value_t fl_module_unique_name(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
 {
     argcount(fl_ctx, "current-julia-module-counter", nargs, 1);
     jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
@@ -277,24 +237,20 @@ static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, ui
         value_t bottom_stack_symbol = fl_applyn(fl_ctx, 1, symbol_value(symbol(fl_ctx, "last")), parsed_method_stack);
         funcname = symbol_name(fl_ctx, bottom_stack_symbol);
     }
-    char buf[(funcname != NULL ? strlen(funcname) : 0) + 20];
+    size_t sz = funcname != NULL ? strlen(funcname) + 32 : 32; // 32 is enough for the suffix
+    char *buf = (char*)alloca(sz);
     if (funcname != NULL && strchr(funcname, '#') == NULL) {
-        jl_mutex_lock_nogc(&m->lock);
-        htable_t *counter_table = m->counter_table;
-        if (counter_table == NULL) {
-            counter_table = rebuild_counter_table(m);
-        }
-        // try to find the function name in the module's counter table, if it's not found, add it
-        if (strhash_get(counter_table, funcname) == HT_NOTFOUND) {
-            strhash_put(counter_table, funcname, (void*)((uintptr_t)HT_NOTFOUND + 1));
+        for (int i = 0; ; i++) {
+            snprintf(buf, sz, "%s##%d", funcname, i);
+            jl_sym_t *sym = jl_symbol(buf);
+            if (jl_get_module_binding(m, sym, 0) == NULL) { // make sure this name is not already taken
+                jl_get_module_binding(m, sym, 1); // create the binding
+                return symbol(fl_ctx, buf);
+            }
         }
-        uint32_t nxt = ((uint32_t)(uintptr_t)strhash_get(counter_table, funcname) - (uintptr_t)HT_NOTFOUND - 1);
-        snprintf(buf, sizeof(buf), "%s##%d", funcname, nxt);
-        strhash_put(counter_table, funcname, (void*)(nxt + 1 + (uintptr_t)HT_NOTFOUND + 1));
-        jl_mutex_unlock_nogc(&m->lock);
     }
     else {
-        snprintf(buf, sizeof(buf), "%d", jl_module_next_counter(ctx->module));
+        snprintf(buf, sz, "%d", jl_module_next_counter(m));
     }
     return symbol(fl_ctx, buf);
 }
@@ -329,7 +285,7 @@ static jl_value_t *scm_to_julia_(fl_context_t *fl_ctx, value_t e, jl_module_t *m
 static const builtinspec_t julia_flisp_ast_ext[] = {
     { "defined-julia-global", fl_defined_julia_global }, // TODO: can we kill this safepoint
     { "nothrow-julia-global", fl_nothrow_julia_global },
-    { "current-julia-module-counter", fl_current_module_counter },
+    { "current-julia-module-counter", fl_module_unique_name },
     { "julia-scalar?", fl_julia_scalar },
     { NULL, NULL }
 };
diff --git a/src/init.c b/src/init.c
@@ -869,7 +869,6 @@ static NOINLINE void _finish_julia_init(JL_IMAGE_SEARCH rel, jl_ptls_t ptls, jl_
         jl_load(jl_core_module, "boot.jl");
         post_boot_hooks();
     }
-    jl_main_module->counter_table = NULL;
 
     if (jl_base_module == NULL) {
         // nthreads > 1 requires code in Base
diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
@@ -225,12 +225,11 @@
                  (if lb (list lb ub) (list ub))
                  (if lb (list lb '(core Any)) '())))))
 
-;; check if x is of the form (method <name>) or (method (outerref <name>))
 (define (is-method? x)
   (if (and (pair? x) (eq? (car x) 'method))
       (let ((name (cadr x)))
-        (if (and (pair? name) (eq? (car name) 'outerref))
-            (let ((name (cadr name)))
+        (if (and (pair? name) (eq? (car name) 'globalref))
+            (let ((name (caddr name)))
               (if (symbol? name)
                   #t
                   #f))
@@ -3553,7 +3552,7 @@ f(x) = yt(x)
 
 (define (convert-lambda lam fname interp capt-sp opaq parsed-method-stack)
   (let ((body (add-box-inits-to-body
-               lam (cl-convert- (cadddr lam) fname lam (table) (table) #f interp opaq parsed-method-stack (table) (vinfo-to-table (car (lam:vinfo lam)))))))
+               lam (cl-convert (cadddr lam) fname lam (table) (table) #f interp opaq parsed-method-stack (table) (vinfo-to-table (car (lam:vinfo lam)))))))
     `(lambda ,(lam:args lam)
        (,(clear-capture-bits (car (lam:vinfo lam)))
         ()
@@ -3646,7 +3645,7 @@ f(x) = yt(x)
                                 (equal? rhs0 '(the_exception)))
                             rhs0
                             (make-ssavalue)))
-                  (rhs  (convert-for-type-decl rhs1 (cl-convert- vt fname lam #f #f #f interp opaq parsed-method-stack (table) locals) #t lam))
+                  (rhs  (convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f #f interp opaq parsed-method-stack (table) locals) #t lam))
                   (ex (cond (closed `(call (core setfield!)
                                            ,(if interp
                                                 `($ ,var)
@@ -3933,14 +3932,14 @@ f(x) = yt(x)
 (define (map-cl-convert exprs fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table)))
   (if toplevel
       (map (lambda (x)
-             (let ((tl (lift-toplevel (cl-convert- x fname lam namemap defined
+             (let ((tl (lift-toplevel (cl-convert x fname lam namemap defined
                                                   (and toplevel (toplevel-preserving? x))
                                                   interp opaq parsed-method-stack globals locals))))
                (if (null? (cdr tl))
                    (car tl)
                    `(block ,@(cdr tl) ,(car tl)))))
            exprs)
-      (map (lambda (x) (cl-convert- x fname lam namemap defined #f interp opaq parsed-method-stack globals locals)) exprs)))
+      (map (lambda (x) (cl-convert x fname lam namemap defined #f interp opaq parsed-method-stack globals locals)) exprs)))
 
 (define (prepare-lambda! lam)
   ;; mark all non-arguments as assigned, since locals that are never assigned
@@ -3955,7 +3954,7 @@ f(x) = yt(x)
        (char=? (string.char str 0) #\#)
        (char-numeric? (string.char str 1))))
 
-(define (cl-convert-- e fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table)))
+(define (cl-convert- e fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table)))
   (if (and (not lam)
            (not (and (pair? e) (memq (car e) '(lambda method macro opaque_closure)))))
       (if (atom? e) e
@@ -3978,7 +3977,7 @@ f(x) = yt(x)
                  (val (if (equal? typ '(core Any))
                           val
                           `(call (core typeassert) ,val
-                                 ,(cl-convert- typ fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))))
+                                 ,(cl-convert typ fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))))
             `(block
                ,@(if (eq? box access) '() `((= ,access ,box)))
                ,undefcheck
@@ -4010,7 +4009,7 @@ f(x) = yt(x)
            e)
           ((=)
            (let ((var (cadr e))
-                 (rhs (cl-convert- (caddr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))
+                 (rhs (cl-convert (caddr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))
              (convert-assignment var rhs fname lam interp opaq parsed-method-stack globals locals)))
           ((local-def) ;; make new Box for local declaration of defined variable
            (let ((vi (get locals (cadr e) #f)))
@@ -4100,14 +4099,14 @@ f(x) = yt(x)
                        ((null? cvs)
                         `(block
                           ,@sp-inits
-                          (method ,(cadr e) ,(cl-convert-
+                          (method ,(cadr e) ,(cl-convert
                                           ;; anonymous functions with keyword args generate global
                                           ;; functions that refer to the type of a local function
                                           (rename-sig-types sig namemap)
                                           fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
                                   ,(let ((body (add-box-inits-to-body
                                                 lam2
-                                                (cl-convert- (cadddr lam2) 'anon lam2 (table) (table) #f interp opaq parsed-method-stack (table)
+                                                (cl-convert (cadddr lam2) 'anon lam2 (table) (table) #f interp opaq parsed-method-stack (table)
                                                             (vinfo-to-table (car (lam:vinfo lam2)))))))
                                      `(lambda ,(cadr lam2)
                                         (,(clear-capture-bits (car vis))
@@ -4119,7 +4118,7 @@ f(x) = yt(x)
                                (newlam    (compact-and-renumber (linearize (car exprs)) 'none 0)))
                           `(toplevel-butfirst
                             (block ,@sp-inits
-                                   (method ,(cadr e) ,(cl-convert- sig fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
+                                   (method ,(cadr e) ,(cl-convert sig fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
                                            ,(julia-bq-macro newlam)))
                             ,@top-stmts))))
 
@@ -4224,7 +4223,7 @@ f(x) = yt(x)
                                (append (map (lambda (gs tvar)
                                               (make-assignment gs `(call (core TypeVar) ',tvar (core Any))))
                                             closure-param-syms closure-param-names)
-                                       `((method #f ,(cl-convert- arg-defs fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
+                                       `((method #f ,(cl-convert arg-defs fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
                                                  ,(convert-lambda lam2
                                                                   (if iskw
                                                                       (caddr (lam:args lam2))
@@ -4284,15 +4283,15 @@ f(x) = yt(x)
                 (block ,@body))))
           ;; remaining `::` expressions are type assertions
           ((|::|)
-           (cl-convert- `(call (core typeassert) ,@(cdr e)) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))
+           (cl-convert `(call (core typeassert) ,@(cdr e)) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))
           ;; remaining `decl` expressions are only type assertions if the
           ;; argument is global or a non-symbol.
           ((decl)
            (cond ((and (symbol? (cadr e))
                        (local-in? (cadr e) lam locals))
                   '(null))
                  (else
-                  (cl-convert-
+                  (cl-convert
                    (let ((ref (binding-to-globalref (cadr e))))
                      (if ref
                          (begin
@@ -4305,21 +4304,17 @@ f(x) = yt(x)
                    fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))))
           ;; `with-static-parameters` expressions can be removed now; used only by analyze-vars
           ((with-static-parameters)
-           (cl-convert- (cadr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))
+           (cl-convert (cadr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))
           (else
            (cons (car e)
                  (map-cl-convert (cdr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))))))))
 
-;; wrapper for `cl-convert--`
-(define (cl-convert- e fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table)))
+;; wrapper for `cl-convert-`
+(define (cl-convert e fname lam namemap defined toplevel interp opaq (parsed-method-stack '()) (globals (table)) (locals (table)))
   (if (is-method? e)
       (let ((name (method-expr-name e)))
-        (cl-convert-- e fname lam namemap defined toplevel interp opaq (cons name parsed-method-stack) globals locals))
-      (cl-convert-- e fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))
-
-(define (cl-convert e fname lam namemap defined toplevel interp opaq (globals (table)) (locals (table)))
-  (let ((parsed-method-stack '()))
-    (cl-convert-- e fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))
+        (cl-convert- e fname lam namemap defined toplevel interp opaq (cons name parsed-method-stack) globals locals))
+      (cl-convert- e fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))
 
 (define (closure-convert e) (cl-convert e #f #f (table) (table) #f #f #f))
 
diff --git a/src/julia.h b/src/julia.h
@@ -715,7 +715,6 @@ typedef struct _jl_module_t {
     int8_t max_methods;
     jl_mutex_t lock;
     intptr_t hash;
-    htable_t *counter_table;
 } jl_module_t;
 
 struct _jl_globalref_t {
diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -948,26 +948,29 @@ STATIC_INLINE jl_ptr_kind_union_t jl_walk_binding_inplace(jl_binding_t **bnd, jl
 }
 #endif
 
+STATIC_INLINE int is10digit(char c) JL_NOTSAFEPOINT
+{
+    return (c >= '0' && c <= '9');
+}
+
 STATIC_INLINE int is_anonfn_typename(char *name)
 {
     if (name[0] != '#' || name[1] == '#')
         return 0;
     char *other = strrchr(name, '#');
-    return other > &name[1] && isdigit(other[1]);
+    return other > &name[1] && is10digit(other[1]);
 }
 
 // Returns true for typenames of anounymous functions that have been canonicalized (i.e.
 // we mangled the name of the outermost enclosing function in their name).
 STATIC_INLINE int is_canonicalized_anonfn_typename(char *name) JL_NOTSAFEPOINT
 {
-    if (name[0] != '#')
-        return 0;
     char *delim = strchr(&name[1], '#');
     if (delim == NULL)
         return 0;
     if (delim[1] != '#')
         return 0;
-    if (!isdigit(delim[2]))
+    if (!is10digit(delim[2]))
         return 0;
     return 1;
 }
diff --git a/src/module.c b/src/module.c
@@ -66,7 +66,6 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui
         jl_module_public(m, name, 1);
         JL_GC_POP();
     }
-    m->counter_table = NULL;
     return m;
 }
 
diff --git a/src/staticdata.c b/src/staticdata.c
@@ -74,8 +74,6 @@ External links:
 #include <stdio.h> // printf
 #include <inttypes.h> // PRIxPTR
 
-#include "support/strhash.h"
-
 #include "julia.h"
 #include "julia_internal.h"
 #include "julia_gcext.h"
diff --git a/src/support/Makefile b/src/support/Makefile
@@ -8,7 +8,7 @@ JCXXFLAGS += $(CXXFLAGS)
 JCPPFLAGS += $(CPPFLAGS)
 JLDFLAGS += $(LDFLAGS)
 
-SRCS := hashing timefuncs ptrhash strhash operators utf8 ios htable bitvector \
+SRCS := hashing timefuncs ptrhash operators utf8 ios htable bitvector \
 	int2str libsupportinit arraylist strtod rle
 ifeq ($(OS),WINNT)
 SRCS += asprintf strptime
diff --git a/src/support/strhash.c b/src/support/strhash.c
diff --git a/src/support/strhash.h b/src/support/strhash.h

Original file line number	Diff line number	Diff line change
`@@ -869,7 +869,6 @@ static NOINLINE void _finish_julia_init(JL_IMAGE_SEARCH rel, jl_ptls_t ptls, jl_`
`869`	`869`	`jl_load(jl_core_module, "boot.jl");`
`870`	`870`	`post_boot_hooks();`
`871`	`871`	`}`
`872`		`- jl_main_module->counter_table = NULL;`
`873`	`872`
`874`	`873`	`if (jl_base_module == NULL) {`
`875`	`874`	`// nthreads > 1 requires code in Base`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,6 @@ JL_DLLEXPORT jl_module_t jl_new_module_(jl_sym_t name, jl_module_t *parent, ui`
`66`	`66`	`jl_module_public(m, name, 1);`
`67`	`67`	`JL_GC_POP();`
`68`	`68`	`}`
`69`		`- m->counter_table = NULL;`
`70`	`69`	`return m;`
`71`	`70`	`}`
`72`	`71`