ClojureScript

Duplicate keys via quoting

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 1.7.228
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

For

#{1 '1}

you get

#{1 1}

Activity

Hide
Peter Schuck added a comment -

This happens for the has-set macro and the hash-set literal. Here's what I get from the repl

cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4 5)
#{1 2 3 4 5}
cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4)
#{1 1 2 2 3 3 4 4}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4}
#{2 1 4 4 3 2 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5}
#{2 1 4 4 3 2 5 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5 '5}
#{2 5 1 4 4 3 2 5 1 3}
cljs.user=> (apply hash-set [1 '1 2 '2 3 '3 4 '4])
#{1 2 3 4}

Calling hash-set as a function gives the correct results. The hash-set macro gives the incorrect results until we have more then 8 elements and uses the fromArray method on PersistentHashSet to build the set instead of creating a literal PersistentArrayMap for the set. The literal notation is incorrect no matter how many elements there are.

Show
Peter Schuck added a comment - This happens for the has-set macro and the hash-set literal. Here's what I get from the repl
cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4 5)
#{1 2 3 4 5}
cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4)
#{1 1 2 2 3 3 4 4}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4}
#{2 1 4 4 3 2 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5}
#{2 1 4 4 3 2 5 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5 '5}
#{2 5 1 4 4 3 2 5 1 3}
cljs.user=> (apply hash-set [1 '1 2 '2 3 '3 4 '4])
#{1 2 3 4}
Calling hash-set as a function gives the correct results. The hash-set macro gives the incorrect results until we have more then 8 elements and uses the fromArray method on PersistentHashSet to build the set instead of creating a literal PersistentArrayMap for the set. The literal notation is incorrect no matter how many elements there are.
Hide
Rohit Aggarwal added a comment -

The underlying problem for both is the same in that a PersistentHashSet is being created directly using a PersistentArrayMap where the keys are the elements from the provided sequence. It manifests itself in two places though.

Show
Rohit Aggarwal added a comment - The underlying problem for both is the same in that a PersistentHashSet is being created directly using a PersistentArrayMap where the keys are the elements from the provided sequence. It manifests itself in two places though.
Hide
Rohit Aggarwal added a comment -

I've taken the approach that if we see a quoted constant then don't create the PersistentHashSet directly and instead go via the fromArray function.

Patch has the fix and a test.

Show
Rohit Aggarwal added a comment - I've taken the approach that if we see a quoted constant then don't create the PersistentHashSet directly and instead go via the fromArray function. Patch has the fix and a test.
Hide
Mike Fikes added a comment -

Attached patch no longer applies to master.

Show
Mike Fikes added a comment - Attached patch no longer applies to master.
Hide
A. R added a comment -

It'd be nice if this patch/ticket also included the following case:

(hash-set "a" \a)
Show
A. R added a comment - It'd be nice if this patch/ticket also included the following case:
(hash-set "a" \a)
Hide
A. R added a comment - - edited

Should we increase the scope of this ticket? The same issue exists for maps:

{'0 "a", 0 "b"}
{\a "a", "a" "b"}

I think one possible solution that solves both, quoting and the char/string, could be to to call emit-str in cljs.compiler on the keys/set-members and only then check for uniqueness. Not sure that's a good idea though.

Doesn't solve the hash-set, array-map macros.

Edit: Related ticket: CLJS-2087

Show
A. R added a comment - - edited Should we increase the scope of this ticket? The same issue exists for maps:
{'0 "a", 0 "b"}
{\a "a", "a" "b"}
I think one possible solution that solves both, quoting and the char/string, could be to to call emit-str in cljs.compiler on the keys/set-members and only then check for uniqueness. Not sure that's a good idea though. Doesn't solve the hash-set, array-map macros. Edit: Related ticket: CLJS-2087
Hide
David Nolen added a comment - - edited

Increasing the scope of tickets is not desirable. Please move to a separate issue and cross-reference thanks.

Show
David Nolen added a comment - - edited Increasing the scope of tickets is not desirable. Please move to a separate issue and cross-reference thanks.
Hide
Mike Fikes added a comment -

Patch no longer applies.

Show
Mike Fikes added a comment - Patch no longer applies.

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated: