Skip to content

fix throws information in ComputedConstant Javadoc#1

Merged
minborg merged 1 commit into
openjdk:computed-constantsfrom
danthe1st:computed-constants-javadoc-fix
Aug 9, 2023
Merged

fix throws information in ComputedConstant Javadoc#1
minborg merged 1 commit into
openjdk:computed-constantsfrom
danthe1st:computed-constants-javadoc-fix

Conversation

@danthe1st

@danthe1st danthe1st commented Aug 8, 2023

Copy link
Copy Markdown
Contributor

The current Javadoc of ComputedConstant#orElse mentions throwing a NoSuchElementException in case the element cannot be bound.
However, the Javadoc also mentions returning the passed value in that case.

If we take a look at the code for that in the computed-constants branch:


and
https://github.com/openjdk/leyden/blob/b9219784cc277417dc112a7fbf652bdc021cf806/src/java.base/share/classes/jdk/internal/constant/AbstractComputedConstant.java#L161C27-L161C27
we can see that rethrow parameter is false and no NoSuchElementException is thrown:
if (rethrow) {
throw new NoSuchElementException(e);
}
return other;

I think the @throws NoSuchElementException should be removed from ComputedConstant#orElse:
https://github.com/openjdk/leyden/blob/b9219784cc277417dc112a7fbf652bdc021cf806/src/java.base/share/classes/java/lang/ComputedConstant.java#L294C19-L294C19

I also reported this here: https://mail.openjdk.org/pipermail/leyden-dev/2023-August/000248.html

@bridgekeeper

bridgekeeper Bot commented Aug 8, 2023

Copy link
Copy Markdown

👋 Welcome back danthe1st! A progress list of the required criteria for merging this PR into computed-constants will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@danthe1st danthe1st marked this pull request as draft August 8, 2023 15:24
@danthe1st danthe1st marked this pull request as ready for review August 8, 2023 15:24

@minborg minborg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@minborg minborg merged commit fc5c547 into openjdk:computed-constants Aug 9, 2023
@danthe1st danthe1st deleted the computed-constants-javadoc-fix branch August 9, 2023 12:15
openjdk Bot pushed a commit that referenced this pull request Sep 12, 2024
openjdk Bot pushed a commit that referenced this pull request Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants