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.

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

🥢 nit(Docs): Edited for clarity #109

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

raxhvl
Copy link
Contributor

@raxhvl raxhvl commented Oct 16, 2024

Nick, I was going through the docs to catch up on the pointer schema, and I want to admit that I love your writing!

This PR adds clarity to the pointer schema overview. I replaced "continuous" to convey the intended meaning (neighboring). I also removed "continuous" when describing a single region, as it goes without saying that a region, as a unit, is inherently together. Here is the before and after:

Values in this schema may address a single continuous region of bytes or an aggregation of non-continuous related regions.

Values in this schema may address a single region of bytes or an aggregation of related, non-contiguous regions.

Also, I have replaced the words "block" and "chunk" (part of whole) with the "cluster" (collection of similar things) when defining a region of bytes.

Copy link
Member

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

Thanks for the clean-up!

(BTW, I'll retract my verbal complaint on the call the other week about the choice of the word "contiguous". I was thinking contiguity is a word best used for grids, but apparently computer scientists have been using that word for decades to refer to bytes ranges, so yeah... good call!)

As such, this schema is specified recursively: a pointer is either a single,
continuous sequence of bytes addresses (a **region**), or it aggregates other
As such, this schema is specified recursively: a pointer is either a reference
to a cluster of contiguous bytes (a **region**), or is an aggregation of other
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
to a cluster of contiguous bytes (a **region**), or is an aggregation of other
to a range of contiguous bytes (a **region**), or is an aggregation of other

How about "range" instead of "cluster" here? To stress that the grouping is nothing more fancy than just one byte after the next

pointers (a **collection**).

## A **region** is a single continuous range of byte addresses
## A **region** addresses a cluster of contiguous bytes in machine state
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## A **region** addresses a cluster of contiguous bytes in machine state
## A **region** addresses a range of contiguous bytes in machine state


For simple allocations (like those that fit into a single word), the
**ethdebug/format/pointer** representation is also quite simple: just a single,
optionally named, continuous chunk of bytes in the machine state.
optionally named, cluster of contiguous bytes in the machine state.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
optionally named, cluster of contiguous bytes in the machine state.
optionally named, sequence of contiguous bytes in the machine state.

Have a synonym I guess ;)

@@ -45,16 +45,16 @@ optionally named, continuous chunk of bytes in the machine state.
</details>

This schema defines the concept of a **region** to be the representation
of the addressing details for a particular block of continuous bytes. Different
of the addressing details for a cluster. Different
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
of the addressing details for a cluster. Different
of the addressing details for a bytes range. Different

data locations use different, location-specific schemas for regions
(since, e.g., stack regions are very different than storage regions). The
**ethdebug/format/pointer/region** schema aggregates these using the
`"location"` field as a polymorphic discriminator.

## A **collection** aggregates other pointers

Other allocations are not so cleanly represented by a single continuous block
of bytes anywhere. In these situations, the **ethdebug/format/pointer**
Other allocations are not so cleanly represented by a single cluster of contiguous
Copy link
Member

Choose a reason for hiding this comment

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

"cluster" here is probably fine if you take my other suggestions, since we'll have used the more specific word "range" or "sequence" enough times that the meaning should be driven home by now, and variety of word choice is always nice to prevent things being boring.

@raxhvl
Copy link
Contributor Author

raxhvl commented Oct 31, 2024

Im putting together a note that covers few more things I have in mind about pointers. I will link it here and then lets take this forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants