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

ListBinaryTag is weakly immutable #1070

Closed
mworzala opened this issue Apr 29, 2024 · 3 comments
Closed

ListBinaryTag is weakly immutable #1070

mworzala opened this issue Apr 29, 2024 · 3 comments

Comments

@mworzala
Copy link

mworzala commented Apr 29, 2024

List stores an immutable view of the input lists, it should do a defensive copy (eg List.copyOf).

var entries = new ArrayList<BinaryTag>();
entries.add(StringBinaryTag.stringBinaryTag("abc"));
entries.add(StringBinaryTag.stringBinaryTag("def"));
var tag = ListBinaryTag.listBinaryTag(BinaryTagTypes.STRING, entries);
entries.clear();
assert tag.size() == 2; // fails
@RealBauHD
Copy link
Contributor

You are right, normally new lists and maps are created for the wrapped immutables, because JEP 269 was introduced in Java 9. I'll take a look at the places where it was forgotten.

@RealBauHD
Copy link
Contributor

It seems that only ListBinaryTag#listBinaryTag uses no copy, have you tested it with the CompoundBinaryTag?

@mworzala
Copy link
Author

Yep you're right sorry, compound wraps it correctly I misread.

@mworzala mworzala changed the title ListBinaryTag and CompoundBinaryTag are weakly immutable ListBinaryTag is weakly immutable Apr 29, 2024
@kashike kashike self-assigned this Apr 29, 2024
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

3 participants