From 2ead82b46362112b2b6922cf9561e288513cc840 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Thu, 10 Apr 2025 22:52:22 -0400 Subject: [PATCH 1/7] Only generate globally unique parameter names for inline functions --- CHANGELOG.md | 2 +- docs/compiler.rst | 10 ++++++ docs/pyinterop.rst | 12 ++----- src/basilisp/core.lpy | 42 ++++++++++++------------- src/basilisp/lang/compiler/analyzer.py | 4 ++- src/basilisp/lang/compiler/constants.py | 2 +- src/basilisp/lang/compiler/generator.py | 28 ++++++++--------- 7 files changed, 52 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d62b3155..efd69e6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added `afor` and `awith` macros to support async Python interop (#1179, #1181) ### Changed - * Single arity functions can be tagged with `^:allow-unsafe-names` to preserve their parameter names (#1212) + * Function parameter names will not be automatically generated with unique suffixes unless the function meta key `^:gen-safe-names` is provided (#1212) ### Fixed * Fix an issue where the compiler would generate an illegal `return` statement for asynchronous generators (#1180) diff --git a/docs/compiler.rst b/docs/compiler.rst index 4475e53b..94020adc 100644 --- a/docs/compiler.rst +++ b/docs/compiler.rst @@ -177,6 +177,16 @@ Functions not meeting these criteria will trigger compile time errors if they ar Users should consider inlining primarily a Basilisp internal feature and use it extremely sparingly in user code. +.. warning:: + + Due to the nature of how inline functions are applied, there is a potential for name clashes between the inline function parameter names and names defined elsewhere in the containing Python module. + Therefore, it is recommended for any inline function to set the meta key ``^:gen-safe-names`` to ensure that the compiler generates globally unique Python parameter names. + For inline functions generated automatically by the compiler, this setting is automatically enabled. + + .. code-block:: + + ^:gen-safe-names (fn [a b] ...) + .. _compiler_debugging: Debugging diff --git a/docs/pyinterop.rst b/docs/pyinterop.rst index 1e09c2a4..b96b38b6 100644 --- a/docs/pyinterop.rst +++ b/docs/pyinterop.rst @@ -22,14 +22,6 @@ When compiled, a ``kebab-case`` identifier always becomes a ``snake_case`` ident The Basilisp compiler munges *all* unsafe Basilisp identifiers to safe Python identifiers, but other cases are unlikely to appear in standard Python interop usage. -.. note:: - - By default, the compiler munges function parameter names and makes them globally unique by appending a monotically increasing number suffix to support function inlining. To enable interop with Python libraries that rely on preserved parameter names, you can use the ```^:allow-unsafe-names`` metadata key to retain the (munged) parameter names. This flag only applies to single-arity Basilisp functions. - - .. code-block:: - - (defn ^:allow-unsafe-names afun [a b] ...) - .. _python_builtins: Python Builtins @@ -316,7 +308,7 @@ These decorators are applied from right to left, similar to how Python decorator ;;; Decorators with arguments, and order of application (right to left) ;; ;; example decorator - (defn mult-x-decorator + (defn mult-x-decorator [x] (fn [f] (fn [] (* (f) x)))) @@ -327,7 +319,7 @@ These decorators are applied from right to left, similar to how Python decorator ;;; defasync support ;; ;; example async decorator - (defn add-7-async-decorator + (defn add-7-async-decorator [f] ^:async (fn [] (+ (await (f)) 7))) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 7b948f81..76ff6400 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -307,27 +307,27 @@ ;; Define inline functions for earlier core functions now that we have all the ;; functionality required to do so. -(.alter-meta #'first assoc :inline (fn [seq] `(basilisp.lang.runtime/first ~seq))) -(.alter-meta #'rest assoc :inline (fn [seq] `(basilisp.lang.runtime/rest ~seq))) -(.alter-meta #'next assoc :inline (fn [seq] `(basilisp.lang.runtime/next ~seq))) -(.alter-meta #'second assoc :inline (fn [seq] `(first (rest ~seq)))) -(.alter-meta #'ffirst assoc :inline (fn [seq] `(first (first ~seq)))) -(.alter-meta #'identity assoc :inline (fn [o] `~o)) -(.alter-meta #'instance? assoc :inline (fn [class obj] `(python/isinstance ~obj ~class))) -(.alter-meta #'boolean? assoc :inline (fn [o] `(instance? python/bool ~o))) -(.alter-meta #'float? assoc :inline (fn [o] `(instance? python/float ~o))) -(.alter-meta #'integer? assoc :inline (fn [o] `(instance? python/int ~o))) -(.alter-meta #'string? assoc :inline (fn [o] `(instance? python/str ~o))) -(.alter-meta #'symbol? assoc :inline (fn [o] `(instance? basilisp.lang.symbol/Symbol ~o))) -(.alter-meta #'keyword? assoc :inline (fn [o] `(instance? basilisp.lang.keyword/Keyword ~o))) -(.alter-meta #'list? assoc :inline (fn [o] `(instance? basilisp.lang.interfaces/IPersistentList ~o))) -(.alter-meta #'map? assoc :inline (fn [o] `(instance? basilisp.lang.interfaces/IPersistentMap ~o))) -(.alter-meta #'set? assoc :inline (fn [o] `(instance? basilisp.lang.interfaces/IPersistentSet ~o))) -(.alter-meta #'vector? assoc :inline (fn [o] `(instance? basilisp.lang.interfaces/IPersistentVector ~o))) -(.alter-meta #'seq? assoc :inline (fn [o] `(instance? basilisp.lang.interfaces/ISeq ~o))) -(.alter-meta #'seq assoc :inline (fn [o] `(basilisp.lang.runtime/to-seq ~o))) -(.alter-meta #'set assoc :inline (fn [coll] `(basilisp.lang.runtime/to-set ~coll))) -(.alter-meta #'vec assoc :inline (fn [coll] `(basilisp.lang.runtime/vector ~coll))) +(.alter-meta #'first assoc :inline ^:gen-safe-names (fn [seq] `(basilisp.lang.runtime/first ~seq))) +(.alter-meta #'rest assoc :inline ^:gen-safe-names (fn [seq] `(basilisp.lang.runtime/rest ~seq))) +(.alter-meta #'next assoc :inline ^:gen-safe-names (fn [seq] `(basilisp.lang.runtime/next ~seq))) +(.alter-meta #'second assoc :inline ^:gen-safe-names (fn [seq] `(first (rest ~seq)))) +(.alter-meta #'ffirst assoc :inline ^:gen-safe-names (fn [seq] `(first (first ~seq)))) +(.alter-meta #'identity assoc :inline ^:gen-safe-names (fn [o] `~o)) +(.alter-meta #'instance? assoc :inline ^:gen-safe-names (fn [class obj] `(python/isinstance ~obj ~class))) +(.alter-meta #'boolean? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/bool ~o))) +(.alter-meta #'float? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/float ~o))) +(.alter-meta #'integer? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/int ~o))) +(.alter-meta #'string? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/str ~o))) +(.alter-meta #'symbol? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.symbol/Symbol ~o))) +(.alter-meta #'keyword? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.keyword/Keyword ~o))) +(.alter-meta #'list? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentList ~o))) +(.alter-meta #'map? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentMap ~o))) +(.alter-meta #'set? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentSet ~o))) +(.alter-meta #'vector? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentVector ~o))) +(.alter-meta #'seq? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/ISeq ~o))) +(.alter-meta #'seq assoc :inline ^:gen-safe-names (fn [o] `(basilisp.lang.runtime/to-seq ~o))) +(.alter-meta #'set assoc :inline ^:gen-safe-names (fn [coll] `(basilisp.lang.runtime/to-set ~coll))) +(.alter-meta #'vec assoc :inline ^:gen-safe-names (fn [coll] `(basilisp.lang.runtime/vector ~coll))) (def ^{:macro true diff --git a/src/basilisp/lang/compiler/analyzer.py b/src/basilisp/lang/compiler/analyzer.py index 436a5657..c1c2a7a9 100644 --- a/src/basilisp/lang/compiler/analyzer.py +++ b/src/basilisp/lang/compiler/analyzer.py @@ -56,6 +56,7 @@ SYM_CLASSMETHOD_META_KEY, SYM_DEFAULT_META_KEY, SYM_DYNAMIC_META_KEY, + SYM_GEN_SAFE_NAMES_META_KEY, SYM_INLINE_META_KW, SYM_KWARGS_META_KEY, SYM_MACRO_META_KEY, @@ -1070,7 +1071,7 @@ def _def_ast( # pylint: disable=too-many-locals,too-many-statements "Cannot have a user-generated inline function and an automatically " "generated inline function" ) - var.meta.assoc(SYM_INLINE_META_KW, init.inline_fn) # type: ignore[union-attr] + var.alter_meta(lambda m: m.assoc(SYM_INLINE_META_KW, init.inline_fn)) # type: ignore[union-attr] def_meta = def_meta.assoc(SYM_INLINE_META_KW, init.inline_fn.form) # type: ignore[union-attr] if tag_ast is not None and any( @@ -2210,6 +2211,7 @@ def _inline_fn_ast( *([sym.symbol(genname(f"{name.name}-inline"))] if name is not None else []), vec.vector(binding.form for binding in inline_arity.params), macroed_form, + meta=lmap.map({SYM_GEN_SAFE_NAMES_META_KEY: True}), ) return _fn_ast(inline_fn_form, ctx) diff --git a/src/basilisp/lang/compiler/constants.py b/src/basilisp/lang/compiler/constants.py index 4ecea5b0..fb751539 100644 --- a/src/basilisp/lang/compiler/constants.py +++ b/src/basilisp/lang/compiler/constants.py @@ -37,7 +37,7 @@ class SpecialForm: SYM_ABSTRACT_META_KEY = kw.keyword("abstract") SYM_ABSTRACT_MEMBERS_META_KEY = kw.keyword("abstract-members") -SYM_ALLOW_UNSAFE_NAMES_META_KEY = kw.keyword("allow-unsafe-names") +SYM_GEN_SAFE_NAMES_META_KEY = kw.keyword("gen-safe-names") SYM_ASYNC_META_KEY = kw.keyword("async") SYM_KWARGS_META_KEY = kw.keyword("kwargs") SYM_PRIVATE_META_KEY = kw.keyword("private") diff --git a/src/basilisp/lang/compiler/generator.py b/src/basilisp/lang/compiler/generator.py index f5f68b37..5cf2d4a8 100644 --- a/src/basilisp/lang/compiler/generator.py +++ b/src/basilisp/lang/compiler/generator.py @@ -37,8 +37,8 @@ INTERFACE_KW, OPERATOR_ALIAS, REST_KW, - SYM_ALLOW_UNSAFE_NAMES_META_KEY, SYM_DYNAMIC_META_KEY, + SYM_GEN_SAFE_NAMES_META_KEY, SYM_REDEF_META_KEY, VAR_IS_PROTOCOL_META_KEY, ) @@ -654,13 +654,11 @@ def with_lineno_and_col( MetaNode = Union[Const, MapNode] -def _is_allow_unsafe_names(fn_meta_node: Optional[MetaNode]) -> bool: - """Return True if the `fn_meta_node` has the meta key set to - retain functio parameter names. - - """ +def _is_gen_safe_names(fn_meta_node: Optional[MetaNode]) -> bool: + """Return True if the `fn_meta_node` has the meta key set to generate globally + unique function parameter names.""" return ( - bool(fn_meta_node.form.val_at(SYM_ALLOW_UNSAFE_NAMES_META_KEY, False)) is True + bool(fn_meta_node.form.val_at(SYM_GEN_SAFE_NAMES_META_KEY, False)) is True if fn_meta_node is not None and isinstance(fn_meta_node.form, IPersistentMap) else False ) @@ -1659,7 +1657,7 @@ def __fn_args_to_py_ast( ctx: GeneratorContext, params: Iterable[Binding], body: Do, - allow_unsafe_param_names: bool = True, + should_generate_safe_names: bool = False, ) -> tuple[list[ast.arg], Optional[ast.arg], list[ast.stmt], Iterable[PyASTNode]]: """Generate a list of Python AST nodes from function method parameters. @@ -1675,7 +1673,7 @@ def __fn_args_to_py_ast( assert binding.init is None, ":fn nodes cannot have binding :inits" assert varg is None, "Must have at most one variadic arg" arg_name = munge(binding.name) - if not allow_unsafe_param_names: + if should_generate_safe_names: arg_name = genname(arg_name) arg_tag: Optional[ast.expr] @@ -1822,14 +1820,12 @@ def __single_arity_fn_to_py_ast( # pylint: disable=too-many-locals sym.symbol(lisp_fn_name), py_fn_name, LocalType.FN ) - # check if we should preserve the original parameter names - allow_unsafe_param_names = _is_allow_unsafe_names(meta_node) - fn_args, varg, fn_body_ast, fn_def_deps = __fn_args_to_py_ast( ctx, method.params, method.body, - allow_unsafe_param_names=allow_unsafe_param_names, + # check if we should preserve the original parameter names + should_generate_safe_names=_is_gen_safe_names(meta_node), ) meta_deps, meta_decorators = __fn_meta(ctx, meta_node) @@ -2138,7 +2134,11 @@ def __multi_arity_fn_to_py_ast( # pylint: disable=too-many-locals ) fn_args, varg, fn_body_ast, fn_def_deps = __fn_args_to_py_ast( - ctx, arity.params, arity.body + ctx, + arity.params, + arity.body, + # check if we should preserve the original parameter names + should_generate_safe_names=_is_gen_safe_names(meta_node), ) all_arity_def_deps.extend(fn_def_deps) From 1c600d28f573584e95a55915ae4ec8fc04cc58c4 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 11 Apr 2025 08:52:10 -0400 Subject: [PATCH 2/7] genname on "_" named args to allow multiple in a single definition --- src/basilisp/lang/compiler/generator.py | 6 +- tests/basilisp/compiler_test.py | 98 +++++++++++-------------- 2 files changed, 49 insertions(+), 55 deletions(-) diff --git a/src/basilisp/lang/compiler/generator.py b/src/basilisp/lang/compiler/generator.py index 5cf2d4a8..ee45c4c1 100644 --- a/src/basilisp/lang/compiler/generator.py +++ b/src/basilisp/lang/compiler/generator.py @@ -1673,7 +1673,11 @@ def __fn_args_to_py_ast( assert binding.init is None, ":fn nodes cannot have binding :inits" assert varg is None, "Must have at most one variadic arg" arg_name = munge(binding.name) - if should_generate_safe_names: + # Always generate a unique name for bindings named "_" since those are + # typically ignored parameters. Python doesn't allow duplicate param + # names (even including "_"), so this is a hack to support something + # Clojure allows. + if should_generate_safe_names or binding.name == "_": arg_name = genname(arg_name) arg_tag: Optional[ast.expr] diff --git a/tests/basilisp/compiler_test.py b/tests/basilisp/compiler_test.py index eb8f2b8f..d3411e90 100644 --- a/tests/basilisp/compiler_test.py +++ b/tests/basilisp/compiler_test.py @@ -2776,6 +2776,50 @@ def test_fn_method_allows_empty_body( assert callable(fvar.value) assert None is fvar.value() + @pytest.mark.parametrize( + "code,args", + [ + ("(defn test_dfn1a [a b] 5)", ["a", "b"]), + ("(def test_dfn2 (fn [a b & c] 5))", ["a", "b", "c"]), + ("(def test_dfn3 (fn [a b {:as c}] 5))", ["a", "b", "c"]), + ], + ) + def test_fn_argument_names_unchanged( + self, lcompile: CompileFn, code: str, args: list[str] + ): + fvar = lcompile(code) + fn_args = list(inspect.signature(fvar.deref()).parameters.keys()) + assert fn_args == args + + @pytest.mark.parametrize( + "code", + [ + "(defn ^:gen-safe-names test_dfn0 [a b] 5)", + "(def ^:gen-safe-names test_dfn2 (fn [a b & c] 5))", + "(def test_dfn2 ^:gen-safe-names (fn [a b & c] 5))", + "(def test_dfn2 ^:gen-safe-names (fn [a b {:as c}] 5))", + ], + ) + def test_fn_argument_names_globally_unique(self, lcompile: CompileFn, code: str): + fvar = lcompile(code) + args = list(inspect.signature(fvar.deref()).parameters.keys()) + assert all( + re.fullmatch(r"[a-z]_\d+", arg) for arg in args + ), f"unexpected argument names {args}" + + def test_fn_argument_names_unchanged_except_ignored_argument( + self, lcompile: CompileFn + ): + code = "(fn [a b _ _] 5)" + fvar = lcompile(code) + fn_args = set(inspect.signature(fvar).parameters.keys()) + assert len(fn_args) == 4 + assert {"a", "b"}.issubset(fn_args) + remaining_args = fn_args.difference({"a", "b"}) + assert all( + re.fullmatch(r"__\d+", arg) for arg in remaining_args + ), f"unexpected argument names {remaining_args}" + def test_single_arity_fn(self, lcompile: CompileFn, ns: runtime.Namespace): code = """ (def string-upper (fn* string-upper [s] (.upper s))) @@ -7222,57 +7266,3 @@ def test_yield_as_coroutine_with_multiple_yields( assert kw.keyword("coroutine-value") == state.deref() assert None is next(coro, None) assert kw.keyword("done") == state.deref() - - -def test_defn_argument_names(lcompile: CompileFn): - # By default, function parameter names are made globally unique. - # - # For example, notice how defn generate parameter names in the - # pattern _ to - # ensure global uniqueness. - code = """ - (defn test_dfn0 [a b] 5) - """ - fvar = lcompile(code) - args = list(inspect.signature(fvar.deref()).parameters.keys()) - assert all( - re.fullmatch(p, s) for p, s in zip([r"a_\d+", r"b_\d+"], args) - ), f"unexpected argument names {args}" - - code = """ - (defn ^:allow-unsafe-names test_dfn1a [a b] 5) - """ - fvar = lcompile(code) - args = list(inspect.signature(fvar.deref()).parameters.keys()) - assert args == ["a", "b"] - - code = """ - (defn ^{:allow-unsafe-names false} test_dfn1b [a b] 5) - """ - fvar = lcompile(code) - args = list(inspect.signature(fvar.deref()).parameters.keys()) - assert all( - re.fullmatch(p, s) for p, s in zip([r"a_\d+", r"b_\d+"], args) - ), f"unexpected argument names {args}" - - code = """ - (def ^:allow-unsafe-names test_dfn2 (fn [a b & c] 5)) - """ - fvar = lcompile(code) - args = list(inspect.signature(fvar.deref()).parameters.keys()) - assert args == ["a", "b", "c"] - - code = """ - (def test_dfn3 ^:allow-unsafe-names (fn [a b & c] 5)) - """ - fvar = lcompile(code) - args = list(inspect.signature(fvar.deref()).parameters.keys()) - assert args == ["a", "b", "c"] - - code = """ - ^{:kwargs :collect} - (defn ^:allow-unsafe-names test_dfn4 [a b {:as xyz}] 5) - """ - fvar = lcompile(code) - args = list(inspect.signature(fvar.deref()).parameters.keys()) - assert args == ["a", "b", "xyz"] From 65ecdb25c6e00c78b5841cb3bd2ade4a6ad425a6 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 11 Apr 2025 08:54:40 -0400 Subject: [PATCH 3/7] Stop tests failing locally maybe --- tests/basilisp/source_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/basilisp/source_test.py b/tests/basilisp/source_test.py index 12c34031..58c6368a 100644 --- a/tests/basilisp/source_test.py +++ b/tests/basilisp/source_test.py @@ -30,6 +30,7 @@ def test_format_source_context(monkeypatch, source_file, source_file_path): """ ) ) + monkeypatch.setenv("TERM", "xterm") format_c = format_source_context(source_file_path, 2, end_line=4) assert [ " 1 | \x1b[37m\x1b[39;49;00m\n", From 577e9c00529f594176c21b9aa14ba703ab610130 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 11 Apr 2025 08:55:23 -0400 Subject: [PATCH 4/7] yeah --- tests/basilisp/source_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/basilisp/source_test.py b/tests/basilisp/source_test.py index 58c6368a..d4a963a9 100644 --- a/tests/basilisp/source_test.py +++ b/tests/basilisp/source_test.py @@ -23,7 +23,7 @@ def test_format_source_context(monkeypatch, source_file, source_file_path): textwrap.dedent( """ (ns source-test) - + (a) (let [a 5] (b)) From 01c456753cd56a412d7fa75a246ae4f0438ba852 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 11 Apr 2025 09:14:34 -0400 Subject: [PATCH 5/7] type check fix --- src/basilisp/lang/compiler/analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basilisp/lang/compiler/analyzer.py b/src/basilisp/lang/compiler/analyzer.py index c1c2a7a9..0a105ff2 100644 --- a/src/basilisp/lang/compiler/analyzer.py +++ b/src/basilisp/lang/compiler/analyzer.py @@ -1071,7 +1071,7 @@ def _def_ast( # pylint: disable=too-many-locals,too-many-statements "Cannot have a user-generated inline function and an automatically " "generated inline function" ) - var.alter_meta(lambda m: m.assoc(SYM_INLINE_META_KW, init.inline_fn)) # type: ignore[union-attr] + var.alter_meta(lambda m: m.assoc(SYM_INLINE_META_KW, init.inline_fn)) # type: ignore[misc] def_meta = def_meta.assoc(SYM_INLINE_META_KW, init.inline_fn.form) # type: ignore[union-attr] if tag_ast is not None and any( From 2c1f38bdde77b08cfed1c3ea4d3558433e96b44e Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 11 Apr 2025 17:04:51 -0400 Subject: [PATCH 6/7] Slightly different/better name for this param --- docs/compiler.rst | 4 +-- src/basilisp/core.lpy | 46 ++++++++++++------------- src/basilisp/lang/compiler/analyzer.py | 10 +++--- src/basilisp/lang/compiler/constants.py | 2 +- src/basilisp/lang/compiler/generator.py | 13 ++++--- src/basilisp/lang/reader.py | 4 +-- tests/basilisp/compiler_test.py | 8 ++--- 7 files changed, 45 insertions(+), 42 deletions(-) diff --git a/docs/compiler.rst b/docs/compiler.rst index 94020adc..34869384 100644 --- a/docs/compiler.rst +++ b/docs/compiler.rst @@ -180,12 +180,12 @@ Functions not meeting these criteria will trigger compile time errors if they ar .. warning:: Due to the nature of how inline functions are applied, there is a potential for name clashes between the inline function parameter names and names defined elsewhere in the containing Python module. - Therefore, it is recommended for any inline function to set the meta key ``^:gen-safe-names`` to ensure that the compiler generates globally unique Python parameter names. + Therefore, it is recommended for any inline function to set the meta key ``^:safe-py-params`` to ensure that the compiler generates globally unique Python parameter names. For inline functions generated automatically by the compiler, this setting is automatically enabled. .. code-block:: - ^:gen-safe-names (fn [a b] ...) + ^:safe-py-params (fn [a b] ...) .. _compiler_debugging: diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 76ff6400..ce481d15 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -307,27 +307,27 @@ ;; Define inline functions for earlier core functions now that we have all the ;; functionality required to do so. -(.alter-meta #'first assoc :inline ^:gen-safe-names (fn [seq] `(basilisp.lang.runtime/first ~seq))) -(.alter-meta #'rest assoc :inline ^:gen-safe-names (fn [seq] `(basilisp.lang.runtime/rest ~seq))) -(.alter-meta #'next assoc :inline ^:gen-safe-names (fn [seq] `(basilisp.lang.runtime/next ~seq))) -(.alter-meta #'second assoc :inline ^:gen-safe-names (fn [seq] `(first (rest ~seq)))) -(.alter-meta #'ffirst assoc :inline ^:gen-safe-names (fn [seq] `(first (first ~seq)))) -(.alter-meta #'identity assoc :inline ^:gen-safe-names (fn [o] `~o)) -(.alter-meta #'instance? assoc :inline ^:gen-safe-names (fn [class obj] `(python/isinstance ~obj ~class))) -(.alter-meta #'boolean? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/bool ~o))) -(.alter-meta #'float? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/float ~o))) -(.alter-meta #'integer? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/int ~o))) -(.alter-meta #'string? assoc :inline ^:gen-safe-names (fn [o] `(instance? python/str ~o))) -(.alter-meta #'symbol? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.symbol/Symbol ~o))) -(.alter-meta #'keyword? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.keyword/Keyword ~o))) -(.alter-meta #'list? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentList ~o))) -(.alter-meta #'map? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentMap ~o))) -(.alter-meta #'set? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentSet ~o))) -(.alter-meta #'vector? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/IPersistentVector ~o))) -(.alter-meta #'seq? assoc :inline ^:gen-safe-names (fn [o] `(instance? basilisp.lang.interfaces/ISeq ~o))) -(.alter-meta #'seq assoc :inline ^:gen-safe-names (fn [o] `(basilisp.lang.runtime/to-seq ~o))) -(.alter-meta #'set assoc :inline ^:gen-safe-names (fn [coll] `(basilisp.lang.runtime/to-set ~coll))) -(.alter-meta #'vec assoc :inline ^:gen-safe-names (fn [coll] `(basilisp.lang.runtime/vector ~coll))) +(.alter-meta #'first assoc :inline ^:safe-py-params (fn [seq] `(basilisp.lang.runtime/first ~seq))) +(.alter-meta #'rest assoc :inline ^:safe-py-params (fn [seq] `(basilisp.lang.runtime/rest ~seq))) +(.alter-meta #'next assoc :inline ^:safe-py-params (fn [seq] `(basilisp.lang.runtime/next ~seq))) +(.alter-meta #'second assoc :inline ^:safe-py-params (fn [seq] `(first (rest ~seq)))) +(.alter-meta #'ffirst assoc :inline ^:safe-py-params (fn [seq] `(first (first ~seq)))) +(.alter-meta #'identity assoc :inline ^:safe-py-params (fn [o] `~o)) +(.alter-meta #'instance? assoc :inline ^:safe-py-params (fn [class obj] `(python/isinstance ~obj ~class))) +(.alter-meta #'boolean? assoc :inline ^:safe-py-params (fn [o] `(instance? python/bool ~o))) +(.alter-meta #'float? assoc :inline ^:safe-py-params (fn [o] `(instance? python/float ~o))) +(.alter-meta #'integer? assoc :inline ^:safe-py-params (fn [o] `(instance? python/int ~o))) +(.alter-meta #'string? assoc :inline ^:safe-py-params (fn [o] `(instance? python/str ~o))) +(.alter-meta #'symbol? assoc :inline ^:safe-py-params (fn [o] `(instance? basilisp.lang.symbol/Symbol ~o))) +(.alter-meta #'keyword? assoc :inline ^:safe-py-params (fn [o] `(instance? basilisp.lang.keyword/Keyword ~o))) +(.alter-meta #'list? assoc :inline ^:safe-py-params (fn [o] `(instance? basilisp.lang.interfaces/IPersistentList ~o))) +(.alter-meta #'map? assoc :inline ^:safe-py-params (fn [o] `(instance? basilisp.lang.interfaces/IPersistentMap ~o))) +(.alter-meta #'set? assoc :inline ^:safe-py-params (fn [o] `(instance? basilisp.lang.interfaces/IPersistentSet ~o))) +(.alter-meta #'vector? assoc :inline ^:safe-py-params (fn [o] `(instance? basilisp.lang.interfaces/IPersistentVector ~o))) +(.alter-meta #'seq? assoc :inline ^:safe-py-params (fn [o] `(instance? basilisp.lang.interfaces/ISeq ~o))) +(.alter-meta #'seq assoc :inline ^:safe-py-params (fn [o] `(basilisp.lang.runtime/to-seq ~o))) +(.alter-meta #'set assoc :inline ^:safe-py-params (fn [coll] `(basilisp.lang.runtime/to-set ~coll))) +(.alter-meta #'vec assoc :inline ^:safe-py-params (fn [coll] `(basilisp.lang.runtime/vector ~coll))) (def ^{:macro true @@ -4796,7 +4796,7 @@ as the result. Returns the new value. .. note:: - + Due to Basilisp's :ref:`Direct Linking Optimization `, changes to a Var's root value may not be reflected in the code unless the Var is dynamic. To ensure updates propagate, set the Var's `^:redef` metadata, which disables @@ -6068,7 +6068,7 @@ generated function from right to left. .. note:: - + The ``name`` metadata (i.e., ``(fn ^{...} ...)``) takes precedence over the ``form`` metadata (i.e., ``^{...} (fn ...)``) when both specify the same key. diff --git a/src/basilisp/lang/compiler/analyzer.py b/src/basilisp/lang/compiler/analyzer.py index 0a105ff2..9eb2e01b 100644 --- a/src/basilisp/lang/compiler/analyzer.py +++ b/src/basilisp/lang/compiler/analyzer.py @@ -56,7 +56,7 @@ SYM_CLASSMETHOD_META_KEY, SYM_DEFAULT_META_KEY, SYM_DYNAMIC_META_KEY, - SYM_GEN_SAFE_NAMES_META_KEY, + SYM_GEN_SAFE_PYTHON_PARAM_NAMES_META_KEY, SYM_INLINE_META_KW, SYM_KWARGS_META_KEY, SYM_MACRO_META_KEY, @@ -1571,7 +1571,7 @@ def __deftype_or_reify_method_node_from_arities( ) -def __deftype_or_reify_impls( # pylint: disable=too-many-branches,too-many-locals # noqa: MC0001 +def __deftype_or_reify_impls( # pylint: disable=too-many-branches,too-many-locals form: ISeq, ctx: AnalyzerContext, special_form: sym.Symbol ) -> tuple[list[DefTypeBase], list[DefTypeMember]]: """Roll up `deftype*` and `reify*` declared bases and method implementations.""" @@ -2211,12 +2211,12 @@ def _inline_fn_ast( *([sym.symbol(genname(f"{name.name}-inline"))] if name is not None else []), vec.vector(binding.form for binding in inline_arity.params), macroed_form, - meta=lmap.map({SYM_GEN_SAFE_NAMES_META_KEY: True}), + meta=lmap.map({SYM_GEN_SAFE_PYTHON_PARAM_NAMES_META_KEY: True}), ) return _fn_ast(inline_fn_form, ctx) -@_with_meta # noqa: MC0001 +@_with_meta def _fn_ast( # pylint: disable=too-many-locals,too-many-statements form: Union[llist.PersistentList, ISeq], ctx: AnalyzerContext ) -> Fn: @@ -3603,7 +3603,7 @@ def __resolve_namespaced_symbol_in_ns( return None -def __resolve_namespaced_symbol( # pylint: disable=too-many-branches # noqa: MC0001 +def __resolve_namespaced_symbol( # pylint: disable=too-many-branches ctx: AnalyzerContext, form: sym.Symbol ) -> Union[Const, HostField, MaybeClass, MaybeHostForm, VarRef]: """Resolve a namespaced symbol into a Python name or Basilisp Var.""" diff --git a/src/basilisp/lang/compiler/constants.py b/src/basilisp/lang/compiler/constants.py index fb751539..d8a10449 100644 --- a/src/basilisp/lang/compiler/constants.py +++ b/src/basilisp/lang/compiler/constants.py @@ -37,7 +37,7 @@ class SpecialForm: SYM_ABSTRACT_META_KEY = kw.keyword("abstract") SYM_ABSTRACT_MEMBERS_META_KEY = kw.keyword("abstract-members") -SYM_GEN_SAFE_NAMES_META_KEY = kw.keyword("gen-safe-names") +SYM_GEN_SAFE_PYTHON_PARAM_NAMES_META_KEY = kw.keyword("safe-py-params") SYM_ASYNC_META_KEY = kw.keyword("async") SYM_KWARGS_META_KEY = kw.keyword("kwargs") SYM_PRIVATE_META_KEY = kw.keyword("private") diff --git a/src/basilisp/lang/compiler/generator.py b/src/basilisp/lang/compiler/generator.py index ee45c4c1..ce7b919d 100644 --- a/src/basilisp/lang/compiler/generator.py +++ b/src/basilisp/lang/compiler/generator.py @@ -38,7 +38,7 @@ OPERATOR_ALIAS, REST_KW, SYM_DYNAMIC_META_KEY, - SYM_GEN_SAFE_NAMES_META_KEY, + SYM_GEN_SAFE_PYTHON_PARAM_NAMES_META_KEY, SYM_REDEF_META_KEY, VAR_IS_PROTOCOL_META_KEY, ) @@ -654,11 +654,12 @@ def with_lineno_and_col( MetaNode = Union[Const, MapNode] -def _is_gen_safe_names(fn_meta_node: Optional[MetaNode]) -> bool: +def _should_gen_safe_python_param_names(fn_meta_node: Optional[MetaNode]) -> bool: """Return True if the `fn_meta_node` has the meta key set to generate globally unique function parameter names.""" return ( - bool(fn_meta_node.form.val_at(SYM_GEN_SAFE_NAMES_META_KEY, False)) is True + bool(fn_meta_node.form.val_at(SYM_GEN_SAFE_PYTHON_PARAM_NAMES_META_KEY, False)) + is True if fn_meta_node is not None and isinstance(fn_meta_node.form, IPersistentMap) else False ) @@ -1829,7 +1830,7 @@ def __single_arity_fn_to_py_ast( # pylint: disable=too-many-locals method.params, method.body, # check if we should preserve the original parameter names - should_generate_safe_names=_is_gen_safe_names(meta_node), + should_generate_safe_names=_should_gen_safe_python_param_names(meta_node), ) meta_deps, meta_decorators = __fn_meta(ctx, meta_node) @@ -2142,7 +2143,9 @@ def __multi_arity_fn_to_py_ast( # pylint: disable=too-many-locals arity.params, arity.body, # check if we should preserve the original parameter names - should_generate_safe_names=_is_gen_safe_names(meta_node), + should_generate_safe_names=_should_gen_safe_python_param_names( + meta_node + ), ) all_arity_def_deps.extend(fn_def_deps) diff --git a/src/basilisp/lang/reader.py b/src/basilisp/lang/reader.py index d9a532d7..93834578 100644 --- a/src/basilisp/lang/reader.py +++ b/src/basilisp/lang/reader.py @@ -1644,7 +1644,7 @@ def _resolve_tagged_literal( raise ctx.syntax_error(e.message).with_traceback(e.__traceback__) from None -def _read_reader_macro(ctx: ReaderContext) -> LispReaderForm: # noqa: MC0001 +def _read_reader_macro(ctx: ReaderContext) -> LispReaderForm: """Return a data structure evaluated as a reader macro from the input stream.""" start = ctx.reader.advance() assert start == "#" @@ -1731,7 +1731,7 @@ def _read_next_consuming_whitespace(ctx: ReaderContext) -> LispReaderForm: return _read_next(ctx) -def _read_next(ctx: ReaderContext) -> LispReaderForm: # noqa: C901 MC0001 +def _read_next(ctx: ReaderContext) -> LispReaderForm: # noqa: C901 """Read the next full form from the input stream.""" reader = ctx.reader char = reader.peek() diff --git a/tests/basilisp/compiler_test.py b/tests/basilisp/compiler_test.py index d3411e90..996871fc 100644 --- a/tests/basilisp/compiler_test.py +++ b/tests/basilisp/compiler_test.py @@ -2794,10 +2794,10 @@ def test_fn_argument_names_unchanged( @pytest.mark.parametrize( "code", [ - "(defn ^:gen-safe-names test_dfn0 [a b] 5)", - "(def ^:gen-safe-names test_dfn2 (fn [a b & c] 5))", - "(def test_dfn2 ^:gen-safe-names (fn [a b & c] 5))", - "(def test_dfn2 ^:gen-safe-names (fn [a b {:as c}] 5))", + "(defn ^:safe-py-params test_dfn0 [a b] 5)", + "(def ^:safe-py-params test_dfn2 (fn [a b & c] 5))", + "(def test_dfn2 ^:safe-py-params (fn [a b & c] 5))", + "(def test_dfn2 ^:safe-py-params (fn [a b {:as c}] 5))", ], ) def test_fn_argument_names_globally_unique(self, lcompile: CompileFn, code: str): From 9c83164557e93b038a86ba94099638599bbc1ae5 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 11 Apr 2025 17:06:11 -0400 Subject: [PATCH 7/7] In the changelog too --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efd69e6f..c11bbe98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added `afor` and `awith` macros to support async Python interop (#1179, #1181) ### Changed - * Function parameter names will not be automatically generated with unique suffixes unless the function meta key `^:gen-safe-names` is provided (#1212) + * Function parameter names will not be automatically generated with unique suffixes unless the function meta key `^:safe-py-params` is provided (#1212) ### Fixed * Fix an issue where the compiler would generate an illegal `return` statement for asynchronous generators (#1180)