basilisp-lang / basilisp

A Clojure-compatible(-ish) Lisp dialect targeting Python 3.9+
https://basilisp.readthedocs.io
Eclipse Public License 1.0
281 stars 8 forks source link

Unclear behavior on alias shadowing #1045

Closed jjttjj closed 1 month ago

jjttjj commented 1 month ago

Hi, thanks for all your work on Basilisp.

I originally asked this on the clojurians slack.

In a basilisp repl, on the latest version) I'm hitting issues where if I require or import something and use an alias name that "shadows" some other existing name of something (perhaps just python module names?) It wont work:

;;; 
(ns myns
  (:require [basilisp.io :as io]))

(io/as-path ".")

  exception: <class 'basilisp.lang.compiler.exception.CompilerException'>
      phase: :analyzing
    message: can't identify aliased form
       form: io/as-path
   location: <REPL Input>:4

(ns myns
  (:require [basilisp.io :as io1]))

(io1/as-path ".")
;=> . ;;works

A different but seemingly adjacent thing I hit is something like this:

  (import gc)
  (defn gc []
    (gc/collect))

  ;;AttributeError: 'function' object has no attribute 'collect'

Here it seems like definitions in a namespace prevent that namespace from being used as an alias. (If you re-import gc later you can call gc/collect again)

It might be that these are expected behaviors. If so, are these behaviors described in the docs anywhere? As noted on slack, it doesn't seem like they're described in the Differences from Clojure or Python Interop pages.

chrisrink10 commented 1 month ago

Hi and thanks for the bug report (and the mention of the Basilisp Clojurians channel of which I was blissfully unaware of 😅 )!

The first one is due to the fact that Python's io is imported into every namespace's underlying module. These conflicts unfortunately arise more frequently with Python than Java due to the short, often-unqualified names of Python builtin modules. I think there's probably a whole host of things that should be done to address this problem, but the two that I favor are:

  1. Documenting all of the default namespace imports. Until then, you can check those with (ns-imports *ns*) or maybe (keys (ns-imports *ns*)).
  2. Issuing a warning when a duplicate alias exists for imports or requires. By default, other warnings are emitted to a null dev debugger. You can enable the logger locally with (using whichever subcommand you choose):
    BASILISP_USE_DEV_LOGGER=true basilisp repl

I suppose we could use some default aliases for the Python modules imported by default, but at the moment I'm not leaning towards that option.

The second issue seems like maybe just expected behavior (right now). You experience the exact same situation with Python code (which the compiled Basilisp code closely matches):

import gc

def gc():
    gc.collect()

gc()  # AttributeError: 'function' object has no attribute 'collect'

At the end of the day, Python modules are a giant dictionary so all these names are in a single namespace. This one I think is probably more easily resolved in the compiler by importing the module with a safe name (as by gensym) and only storing the alias in the namespace. So then we'd be generating something like this:

import gc as gc_23

def gc():
    gc_23.collect()

gc()  # Success!