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
8260606: Update Valhalla core-libs naming for methods related to primitive classes #318
8260606: Update Valhalla core-libs naming for methods related to primitive classes #318
Conversation
👋 Welcome back rriggs! A progress list of the required criteria for merging this PR into |
@RogerRiggs This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
* | ||
* @return {@code true} if this class is an inline class | ||
* @return {@code true} if this class is a primitive class, otherwise false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: s/false/{@code false}
/
* is returned. | ||
*/ | ||
private Class<?>[] newProjectionTypeArray() { | ||
private Class<?>[] newPrimitiveTypeArray() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps rename to newTypeArrayForPrimitiveClass
@@ -552,17 +552,17 @@ private static LinkageError newLinkageError(Throwable e) { | |||
}; | |||
|
|||
/** | |||
* Invoke the bootstrap methods hashCode for the given inline object. | |||
* Invoke the bootstrap methods hashCode for the given primitive classobject. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a space is missing in classobject
* @param o the instance to hash. | ||
* @return the hash code of the given inline object. | ||
* @return the hash code of the given primitive classobject. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
space is missing in the word classobject
* @param o the instance to hash. | ||
* @return the string representation of the given inline object. | ||
* @return the string representation of the given primitive classobject. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same typo classobject
// return an uninitialized default value if null | ||
ref = uninitializedDefaultValue(vc); | ||
ref = uninitializedDefaultValue(pc); | ||
} | ||
return ref; | ||
} | ||
|
||
public Object getReferenceVolatile(Object o, long offset, Class<?> vc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vc
parameter should also be renamed to pc
to be consistent with other methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Do you want to add IdentityObject
in a separate PR? It's okay with me to add this in this PR.
@RogerRiggs this pull request can not be integrated into git checkout 8251554-rename-primitiveclass
git fetch https://git.openjdk.java.net/valhalla lworld
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge lworld"
git push |
IdentityObject is already in java.lang. java.lang.PrimitiveObject has not yet been added. |
I meant |
…251554-rename-primitiveclass
/integrate |
@RogerRiggs Since your change was applied there have been 233 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit 5f1e8d9. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Update terminology used to refer to primitive classes (from inline classes).
Drop the descriptions using "projection" and use "conversion" as needed.
Updated dependencies on Class.isPrimitiveClass().
[Updating test names will be a separate task.]
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/valhalla pull/318/head:pull/318
$ git checkout pull/318