jdk icon indicating copy to clipboard operation
jdk copied to clipboard

Published 20 hours ago • verified publisher icon openjdk

8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException

Open cushon opened this issue 1 year ago • 14 comments
trafficstars

This change overrides mutator methods in the implementation returned by Map.of().entrySet() to throw UnsupportedOperationException.

Progress

  • [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue
  • [ ] Change requires a CSR request matching fixVersion 24 to be approved (needs to be created)

Issue

  • JDK-8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException (Bug - P4)

Contributors

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18522/head:pull/18522
$ git checkout pull/18522

Update a local copy of the PR:
$ git checkout pull/18522
$ git pull https://git.openjdk.org/jdk.git pull/18522/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18522

View PR using the GUI difftool:
$ git pr show -t 18522

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18522.diff

Webrev

Link to Webrev Comment

cushon avatar Mar 27 '24 17:03 cushon

:wave: Welcome back cushon! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

bridgekeeper[bot] avatar Mar 27 '24 17:03 bridgekeeper[bot]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

openjdk[bot] avatar Mar 27 '24 17:03 openjdk[bot]

@cushon The following label will be automatically applied to this pull request:

  • core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

openjdk[bot] avatar Mar 27 '24 17:03 openjdk[bot]

/contributor add Sorin Basca [email protected]

cushon avatar Mar 27 '24 17:03 cushon

@cushon Contributor Sorin Basca <[email protected]> successfully added.

openjdk[bot] avatar Mar 27 '24 17:03 openjdk[bot]

Webrevs

mlbridge[bot] avatar Mar 28 '24 16:03 mlbridge[bot]

/csr needed

cushon avatar Mar 28 '24 16:03 cushon

@cushon an approved CSR request is already required for this pull request.

openjdk[bot] avatar Mar 28 '24 16:03 openjdk[bot]

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Apr 25 '24 23:04 bridgekeeper[bot]

Please keep this open

cushon avatar Apr 26 '24 18:04 cushon

@stuart-marks Can you take a look at this?

liach avatar May 10 '24 01:05 liach

Sorry for the delay, I'm still totally backlogged on other stuff. I can't commit to get this into JDK 23.

stuart-marks avatar May 24 '24 17:05 stuart-marks

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Jul 02 '24 21:07 bridgekeeper[bot]

I've had a look here, and it functionally looks good to me. The remaining challenge is the impact of the change on existing code (likely written between 11 and now, as <10 code would not have the problem AFAICT). /cc @liach

viktorklang-ora avatar Jul 03 '24 09:07 viktorklang-ora

@cushon Given we have specified in Collection that unmodifiability extends to all derived collection views: https://github.com/openjdk/jdk/blob/4c9a511f75174a3f3ae575fae2677c198c607b52/src/java.base/share/classes/java/util/Collection.java#L159-L160

Would you mind changing "Calling any mutator method on the Map will ..." to something like "... on the map or any derived view collection will ..." to emphasize our new consistency? (Our internal conversation agreed on this) https://github.com/openjdk/jdk/blob/4c9a511f75174a3f3ae575fae2677c198c607b52/src/java.base/share/classes/java/util/Map.java#L125-L126

liach avatar Jul 09 '24 22:07 liach

Would you mind changing "Calling any mutator method on the Map will ..." to something like "... on the map or any derived view collection will ..." to emphasize our new consistency? (Our internal conversation agreed on this)

Done! Kevin made a similar note in the CSR. I have updated the CSR to mention the javadoc change.

cushon avatar Jul 09 '24 22:07 cushon

All three files can have their copyright year updated.

Done

cushon avatar Jul 09 '24 22:07 cushon

Please see the analysis I just added to the bug report:

https://bugs.openjdk.org/browse/JDK-8328821?focusedId=14688968&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14688968

stuart-marks avatar Jul 11 '24 06:07 stuart-marks

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Aug 08 '24 07:08 bridgekeeper[bot]

keep-alive while we evaluate the impact...

liach avatar Aug 08 '24 13:08 liach

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Sep 05 '24 14:09 bridgekeeper[bot]

keep-alive, I still want to see if we can evaluate the impact enough to find a path forward for this

cushon avatar Sep 10 '24 18:09 cushon

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Oct 08 '24 18:10 bridgekeeper[bot]

Please keep this open for now

cushon avatar Oct 08 '24 18:10 cushon

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Nov 05 '24 23:11 bridgekeeper[bot]

Let's keep this open for now

cushon avatar Nov 05 '24 23:11 cushon

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Dec 04 '24 00:12 bridgekeeper[bot]

Please keep open

cushon avatar Dec 04 '24 00:12 cushon

@cushon This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Jan 01 '25 04:01 bridgekeeper[bot]

Let's keep this open

cushon avatar Jan 02 '25 16:01 cushon