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

invalid map entry: nil #261

Open
frenchy64 opened this issue Feb 11, 2025 · 3 comments
Open

invalid map entry: nil #261

frenchy64 opened this issue Feb 11, 2025 · 3 comments

Comments

@frenchy64
Copy link
Contributor

;; jank
clojure.core=> (conj {} nil)
Exception: invalid map entry: nil

;; clojure
user=> (conj {} nil)
{}
@frenchy64
Copy link
Contributor Author

Also related, certain map types cannot be merged due to hardcoding.

clojure.core=> (merge {} (sorted-map))
Exception: invalid map entry: {}

@frenchy64
Copy link
Contributor Author

frenchy64 commented Feb 12, 2025

I looked into this. One solution could be to pull out a common helper to define conj in terms of assoc and fix the problems there. Maybe it could even go in base_persistent_map. Something like this:

  object_ptr conj_map_like(object_ptr const m, object_ptr const head)
  {
    // fix (conj {} nil)
    if(head == obj::nil::nil_const())
    {
      return m;
    }

    // fix (merge {} (sorted-map))
    if(is_map(head->type))
    {
      return runtime::merge(m, head);
    }

    if(head->type != object_type::persistent_vector)
    {
      throw std::runtime_error{ fmt::format("invalid map entry: {}", runtime::to_string(head)) };
    }

    auto const vec(expect_object<obj::persistent_vector>(head));
    if(vec->count() != 2)
    {
      throw std::runtime_error{ fmt::format("invalid map entry: {}", runtime::to_string(head)) };
    }

    return assoc(m, vec->data[0], vec->data[1]);
  }

@jeaye
Copy link
Member

jeaye commented Feb 12, 2025

Also related, certain map types cannot be merged due to hardcoding.

clojure.core=> (merge {} (sorted-map))
Exception: invalid map entry: {}

This code predates behavior::map_like, which we now have. merge should just be updated to use that. We may want a similar concept for map transients, such as behavior::map_transient_like.

@jeaye jeaye removed the type:bug label Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants