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

IntObjectHashMap.values().toArray() method throws ClassCastException #13761

Closed
bugmakerrrrrr opened this issue Sep 11, 2024 · 7 comments · Fixed by #13876 or #13884
Closed

IntObjectHashMap.values().toArray() method throws ClassCastException #13761

bugmakerrrrrr opened this issue Sep 11, 2024 · 7 comments · Fixed by #13876 or #13884
Assignees
Labels

Comments

@bugmakerrrrrr
Copy link
Contributor

Description

The ClassCastEx is thrown when the IntObjectHashMap.values().toArray() method is called with a type other than Object, and of course the same exception occurs with CharObjectHashMap and LongObjectHashMap. The issue can be reproduced with the following test.

@Test
  public void testStringMapValues() {
    IntObjectHashMap<String> map = new IntObjectHashMap<>();
    map.put(1, "a");
    map.put(2, "b");
    map.put(3, "c");
    String[] values = map.values().toArray();
  }

There are several options to fix the issue.

  1. change the return value to Object[];
  2. mimic the ArrayList.toArray(T[]) method to pass a T[] parameter;
  3. replace the toArray method with a asList method, and return a List<T>.

IMO, either option 2 or 3 would be acceptable. Any thought?

Version and environment details

No response

@BrianWoolfolk
Copy link

Has this issue been approved or followed up? I've been testing with this example and also noted a few things:

  1. var values = map.values().toArray() throws ClassCastEx, but Object[] values = ... don't. I guess it just skips the String casting? But seems like an internal casting is also taking place inside .toArray(), but "reverts" back to Object[].
  2. With the test provided, the resulting array is ["a", "c," "b"] which I found counterintuitive given the map.put(2, "b") (this might be some hashing side effect).
  3. I like the idea of the asList() method because it could also address point 2

I'm not that familiar with Java tricks yet, and I would like to research a little bit more on this and make a PR

@dweiss
Copy link
Contributor

dweiss commented Oct 7, 2024

This is indeed a bug. I'm not sure how it got introduced because toArray would return Object[] in HPPC - it only changes to a an array of primitives if the type is primitive:

  /**
   * Default implementation of copying to an array.
   */
  @Override
  /*!#if ($TemplateOptions.KTypePrimitive)
  public KType [] toArray()
     #else !*/
  public Object[] toArray()
  /*! #end !*/

I also compared the code in Lucene with that in HPPC and I see some manual changes there. @bruno-roustant - I guess you ported this partially manually? I wonder if it'd be possible to create a port automatically or at least semi-automatically so that the code doesn't diverge too much from HPPC?

@dweiss dweiss self-assigned this Oct 7, 2024
@dweiss
Copy link
Contributor

dweiss commented Oct 7, 2024

@BrianWoolfolk - (1) it's because of something called type erasure. All the VType types inside toArray are replaced with an Object. Externally, the compiler is mislead to think it'll return a String[] but it is in fact an Object[] instance (which is a different type). (2) associative arrays have the freedom of ordering their elements in any way you like (unless the order is explicitly preserved, like with LinkedHashMap, but this is not the case here).

This method should have the return type of Object[]. Alternatively, it could be removed entirely since it's already an Iterable so for-type loops will work perfectly well.

@bruno-roustant
Copy link
Contributor

@dweiss it is a bit old in my memory, but I remember I had to diverge a bit to avoid taking too many classes from HPPC by dependency. I see that you assigned it to you, and I can take it or help if you don't have time. Thanks

@dweiss
Copy link
Contributor

dweiss commented Oct 7, 2024

Sure, no problem at all! I'll handle this issue and I have a side note to check and see if we can make the port more aligned with HPPC code.

@dweiss
Copy link
Contributor

dweiss commented Oct 9, 2024

Thank you for reporting the problem, @bugmakerrrrrr

@bugmakerrrrrr
Copy link
Contributor Author

@dweiss I think that CharObjectHashMap and LongObjectHashMap need to be fixed as well, I can help to create a PR.

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