Skip to content

Conversation

@adinn
Copy link
Collaborator

@adinn adinn commented Jul 28, 2023

This pull request factors out into interface DwarfConstants all the constant values defined in the DWARF spec that are used by the debug info generator code driven via the DebugInfoProvider API. This is to allow those same constants to be reused by code that needs to manage DWARF info but is not driven via DebugInfoProvider.

The change also cleans up some of the code in the current DebugInfoProvider DWARF backend, removing prefix DW_ from a few constants that are not defined by the DWARF spec and adopting an enum type for the abbreviation codes used to tag DWARF DIEs.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jul 28, 2023
@adinn adinn force-pushed the refactor_dwarf_constants branch from c903dbc to cab0c76 Compare July 30, 2023 09:54
@adinn adinn requested a review from olpaw July 31, 2023 08:00
@adinn
Copy link
Collaborator Author

adinn commented Jul 31, 2023

This is a first step towards factoring out common DWARF management routines as requested in feature request issue #6207.

@adinn adinn requested a review from fniephaus July 31, 2023 09:15
@adinn adinn force-pushed the refactor_dwarf_constants branch 2 times, most recently from 14dff63 to 74dc839 Compare August 2, 2023 11:06
@adinn
Copy link
Collaborator Author

adinn commented Aug 2, 2023

@olpaw @fniephaus I pushed a new change to use enums instead of interfaces for the different categories of DWARF constant. So, this is ready for review again (the remaining gate style problem is unrelated to the patch).

@adinn
Copy link
Collaborator Author

adinn commented Aug 3, 2023

Once again, the gate failure is a spurious error unrelated to this patch.

Copy link
Member

@fniephaus fniephaus left a comment

Choose a reason for hiding this comment

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

LGTM.

@fniephaus fniephaus changed the title Refactor dwarf constants [GR-47755] Refactor dwarf constants Aug 4, 2023
@fniephaus
Copy link
Member

fniephaus commented Aug 4, 2023

Once again, the gate failure is a spurious error unrelated to this patch.

It seems you were still working on an older commit. We've fixed this some time ago with 56671ad. Rebasing to the latest master commit would've fixed the gate in your PR. But no need to waste free computing resources, I try to get it merged the way it is :)

@fniephaus
Copy link
Member

@adinn
Copy link
Collaborator Author

adinn commented Aug 4, 2023

Aaargh, so the problem snuck back into my commit. Do you want me to push a fix?

@fniephaus
Copy link
Member

No, it's fine. I've edited your commits to speed things up.

Copy link
Member

@olpaw olpaw left a comment

Choose a reason for hiding this comment

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

Please replace all the individual import static of enum values with single regular imports of the respective enum classes.

@adinn
Copy link
Collaborator Author

adinn commented Aug 7, 2023

I have pushed a patch that 1) makes all the enum values private and 2) removes all static enum value imports, instead resorting to enum type qualified mentions at the point of use. I think that addresses all the review comments.

@adinn adinn force-pushed the refactor_dwarf_constants branch from 90f489b to 5d7352a Compare August 8, 2023 08:26
@adinn
Copy link
Collaborator Author

adinn commented Aug 8, 2023

Rebased over latest master

@fniephaus
Copy link
Member

Rebased over latest master

Thanks! HybridLayoutSupport.java is still modified in 2ceefd5#diff-6f4e2943f89b429e13da3d3264be1c672561950b119cfabd34467382634d1929 but I'll take care of it...the PR is now in the merge queue and (unfortunately) duplicated at #7158.

@fniephaus
Copy link
Member

Merged via #7158 (comment).

@fniephaus fniephaus closed this Aug 9, 2023
@adinn adinn deleted the refactor_dwarf_constants branch August 9, 2023 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement. redhat-interest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants