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

Interface incompatibility in BiVariableMap on Java 8 #858

Closed
headius opened this Issue Jul 6, 2013 · 3 comments

Comments

Projects
None yet
2 participants
@headius
Copy link
Member

headius commented Jul 6, 2013

BiVariableMap implements Map<K,V> and an additional method called remove:

public V remove(Object receiver, Object key) {
...
}

However, Java 8 has added an additional [remove signature in Map](http://download.java.net/jdk8/docs/api/java/util/Map.html#remove(java.lang.Object, java.lang.Object))

I'm not sure what all this method is being used for. It has a couple internal uses, but nothing in existing tests, so perhaps it can be renamed for compatibility.

I believe this will not cause existing builds to be broken on Java 8, since anyone compiling against this particular method will have the proper signature embedded in their bytecode, and JVM bytecode can dispatch to methods of the same name and parameter types with different return values.

Copying @yokolet and @bnagy.

@yokolet

This comment has been minimized.

Copy link
Member

yokolet commented Jul 8, 2013

@headius thanks for filing the problem. Test for embedding API is not enough. As far as I remember, the method is for clean-up purpose and users may use it.

But, anyway, I should look at and do something on this method.

headius added a commit that referenced this issue Jul 9, 2013

Temporarily rename problematic remove method to removeFrom.
Final decision about how to fix this still remains with @yokolet

See #858.
@headius

This comment has been minimized.

Copy link
Member Author

headius commented Sep 3, 2013

@yokolet PING! :-)

If the renamed I did is acceptable to you, we can go forward with it.

@yokolet

This comment has been minimized.

Copy link
Member

yokolet commented Sep 3, 2013

(got the ping)

Yes. The renamed method should work. Go forward!

@headius headius closed this Oct 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.