Skip to content

Conversation

@credo-quia-absurdum
Copy link
Contributor

This change improves the readability of comments in gtlist.h.

Previously, the comment columns were visually aligned by placing commas in the
same positions as the actual argument separators. While this kept the columns
technically aligned, the presence of "floating commas" in the comments made the
structure harder to parse at a glance.

The updated version replaces those comma placeholders with clearer alignment
cues, making the layout easier to understand.

No behavioral changes.

Part of #84834, cc @dotnet/samsung
@SkyShield @namu-lee

Copilot AI review requested due to automatic review settings November 19, 2025 10:32
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Nov 19, 2025
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 19, 2025
Copilot finished reviewing on behalf of credo-quia-absurdum November 19, 2025 10:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the visual alignment of comments in gtlist.h by replacing comma-based placeholders with a clearer column alignment scheme using pipe characters (|) and arrows (v). The new format makes the comment structure more readable by explicitly showing the vertical alignment between the column headers and the actual GTNODE macro parameters.

Key Changes:

  • Replaced floating commas in comments with explicit pipe characters (|) to show column boundaries
  • Added arrows (v) to clearly indicate which comment header corresponds to which macro parameter
  • Improved visual clarity while maintaining the same informational content

@am11 am11 added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 19, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Comment on lines 12 to 21
// |
// | , GenTree struct flavor
// | |
// | | ,commutative
// | | |
// | | |,illegal as VN func
// | | | |
// | | | |,(oper kind | DEBUG oper kind)
// | | | | |
// v v v v v
Copy link
Member

Choose a reason for hiding this comment

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

I do not personally find this to be more readable, rather I find it visually distracting instead

I think it would be less problematic if this were just repeating the commas down or instead made each column bigger so all "column headers" could be on one line

but I also think this is fine as it is and can probably avoid unnecessary churn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review and suggestions. I've updated the formatting by repeating the commas down from the existing comma, as you recommended.

If you still feel this change introduces unnecessary churn, please feel free to close the PR. For context, as a newcomer to .NET, the original formatting was a bit hard for me to grasp at first glance, so I tried adjusting it for readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants