Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

namespace should be nil for unqualified idents #254

Closed
frenchy64 opened this issue Feb 8, 2025 · 4 comments · Fixed by #256
Closed

namespace should be nil for unqualified idents #254

frenchy64 opened this issue Feb 8, 2025 · 4 comments · Fixed by #256

Comments

@frenchy64
Copy link
Contributor

frenchy64 commented Feb 8, 2025

;; jank
clojure.core=> (namespace 'a)
""
clojure.core=> (namespace :a)
""

;; clojure
user=> (namespace 'a)
nil
user=> (namespace :a)
nil
user=> (namespace (symbol "" "f")) ;;subtle
""


;; via clojure-test-suite
Testing clojure.core-test.namespace

FAIL in (nil) (nil:nil)
expected: (= nil (namespace :abc))
  actual: (not (= nil ""))

FAIL in (nil) (nil:nil)
expected: (= nil (namespace (quote abc)))
  actual: (not (= nil ""))
Exception: not nameable: nil
@frenchy64
Copy link
Contributor Author

I think changing get_namespace() to return an option<native_persistent_string> will emulate Clojure the closest, where is_none() means unqualified and is_some() means qualified.

The main wrinkle here is that (qualified-symbol? (symbol "" "f")) is true in Clojure.

We might want to more carefully manage ns to ensure it's not empty. It might be a good idea when creating unqualified symbols and keywords to avoid the 1-arity ctor and use make_box<symbol>(none, name). It's probably faster too.

@jeaye
Copy link
Member

jeaye commented Feb 10, 2025

Hm, that sounds like it'll be very easy to get wrong. So Clojure actually distinguishes "" from nil for a symbol's namespace? That's absolutely bonkers.

@frenchy64
Copy link
Contributor Author

frenchy64 commented Feb 10, 2025

My take is that the behavior for a "" namespace is probably unspecified, so it might be ok to co-opt it as unqualified.

Assuming that, a much less invasive change would simply to add a not-empty to the return of namespace.

@jeaye
Copy link
Member

jeaye commented Feb 10, 2025

My take is that the behavior for a "" namespace is probably unspecified, so it might be ok to co-opt it as unqualified.

Assuming that, a much less invasive change would simply to add a not-empty to the return of namespace.

Yep, I like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants