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.

Sign up for GitHub

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

Jump to bottom

Clean up ConstantPoolClassSlotIterator class #20492

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Clean up ConstantPoolClassSlotIterator class #20492

merged 1 commit into from
Nov 14, 2024

Conversation

LinHu2016
Copy link
Contributor

Use consistent data types and code formatting

dmitripivkine
dmitripivkine reviewed Nov 1, 2024
View reviewed changes
runtime/gc_structs/ConstantPoolClassSlotIterator.cpp Outdated
@@ -47,11 +47,11 @@ GC_ConstantPoolClassSlotIterator::nextSlot()
_cpDescriptionIndex = J9_CP_DESCRIPTIONS_PER_U32;
}

U_32 slotType = _cpDescription & J9_CP_DESCRIPTION_MASK;
slotType = _cpDescription & J9_CP_DESCRIPTION_MASK;
Copy link
Contributor

Choose a reason for hiding this comment

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

missed declaration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is also U_8 need to be replaced with uint8_t

amicic
amicic reviewed Nov 2, 2024
View reviewed changes
runtime/gc_structs/ConstantPoolClassSlotIterator.hpp Outdated
_cpEntryCount(clazz->romClass->ramConstantPoolCount)
GC_ConstantPoolClassSlotIterator(J9Class *clazz)
: _cpEntry((J9Object **)J9_CP_FROM_CLASS(clazz))
, _cpEntryCount(clazz->romClass->ramConstantPoolCount)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should preserve indentation

amicic
amicic reviewed Nov 2, 2024
View reviewed changes
runtime/gc_structs/ConstantPoolClassSlotIterator.hpp Outdated
_cpEntryCount(clazz->romClass->ramConstantPoolCount)
GC_ConstantPoolClassSlotIterator(J9Class *clazz)
: _cpEntry((J9Object **)J9_CP_FROM_CLASS(clazz))
, _cpEntryCount(clazz->romClass->ramConstantPoolCount)
{
_cpEntryTotal = _cpEntryCount;
if(_cpEntryCount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing space in 'if('

amicic
amicic reviewed Nov 5, 2024
View reviewed changes
runtime/gc_structs/ConstantPoolClassSlotIterator.hpp Outdated
_cpDescriptionIndex = 0;
}

};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't typically have semicolon after constructor

@dmitripivkine
Copy link
Contributor

jenkins compile win jdk11

@LinHu2016 LinHu2016 force-pushed the coding-std-update5 branch 2 times, most recently from c1c5ec7 to a67e489 Compare November 8, 2024 16:55
@LinHu2016
Copy link
Contributor Author

@dmitripivkine could you please relaunch jenkins compile win jdk11? there was mistake for removing wrong semicolon. Thanks

@amicic
Copy link
Contributor

amicic commented Nov 8, 2024

jenkins compile win jdk11

amicic
amicic reviewed Nov 8, 2024
View reviewed changes
runtime/gc_structs/ConstantPoolClassSlotIterator.hpp Outdated
{
_cpEntryTotal = _cpEntryCount;
if(_cpEntryCount) {
_cpDescriptionSlots = SRP_PTR_GET(&clazz->romClass->cpShapeDescription, U_32 *);
if (_cpEntryCount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

_cpEntryCount is an int, should we do 0 != .... ? @dmitripivkine

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, agreed

dmitripivkine
dmitripivkine reviewed Nov 11, 2024
View reviewed changes
runtime/gc_structs/ConstantPoolClassSlotIterator.cpp Outdated
J9Object **slotPtr = _cpEntry;

/* Adjust the CP slot and description information */
_cpEntry = (J9Object **)( ((U_8 *)_cpEntry) + sizeof(J9RAMConstantPoolItem) );
_cpEntry = (J9Object **)( ((uint8_t *)_cpEntry) + sizeof(J9RAMConstantPoolItem));
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove space in the cast:
(J9Object **)( ((uint8_t *) -> (J9Object **)(((uint8_t *)

@LinHu2016 LinHu2016 force-pushed the coding-std-update5 branch 2 times, most recently from a30f553 to 04e35a2 Compare November 12, 2024 15:24
@amicic
Copy link
Contributor

amicic commented Nov 12, 2024

jenkins compile win jdk11

@amicic amicic added the comp:gc label Nov 12, 2024
@amicic
Copy link
Contributor

amicic commented Nov 12, 2024

hm, seems like you lost some of the chagnes. all old INT types are back

@LinHu2016
Clean up ConstantPoolClassSlotIterator class
8f14967 
Use consistent data types and code formatting

Signed-off-by: lhu <linhu@ca.ibm.com>
@LinHu2016
Copy link
Contributor Author

hm, seems like you lost some of the chagnes. all old INT types are back

there was some rebase issue, I have updated this PR to latest, Thanks

@amicic
Copy link
Contributor

amicic commented Nov 14, 2024

jenkins compile win jdk11

@amicic amicic merged commit 741422b into eclipse-openj9:master Nov 14, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmitripivkine dmitripivkine dmitripivkine approved these changes

@amicic amicic amicic approved these changes

Assignees

@LinHu2016 LinHu2016

Labels
comp:gc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LinHu2016 @dmitripivkine @amicic