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

Style notes on passing and storing object addresses #4310

Merged
merged 4 commits into from
Sep 20, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Sep 13, 2024

Context. Some relevant Google C++ style is at Inputs and Outputs. #4301 is an example application of the style.

@github-actions github-actions bot added the documentation An issue or proposed change to our documentation label Sep 13, 2024
Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

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

LGTM

@jonmeow jonmeow changed the title Brief note on unowned objects Brief style note on unowned objects Sep 16, 2024
Comment on lines 150 to 152
- When storing unowned objects as members, prefer to pass by reference and
store as a pointer. For example, `Foo* foo_;` and
`Bar(Foo& foo) : foo_(&foo)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on further discussion, let's keep the storing part but add some more explicit guidance for passing. This is allowed and slightly encouraged in the Google style guide, but I think we can be a bit more direct and explicit for Carbon:

Suggested change
- When storing unowned objects as members, prefer to pass by reference and
store as a pointer. For example, `Foo* foo_;` and
`Bar(Foo& foo) : foo_(&foo)`.
- When passing a reference to an unowned object as an argument, use a reference
unless one of the following cases applies:
- If it is optional, use a pointer and document that it may be null.
- If it is captured and must outlive the call expression itself, use a
pointer an document that it must not be null (unless it is also optional).
- When storing a reference to an unowned object as a member, prefer to store as a
pointer. For example, `Foo* foo_;` and `Bar(Foo* foo) : foo_(foo)`.

Copy link
Contributor Author

@jonmeow jonmeow Sep 20, 2024

Choose a reason for hiding this comment

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

Pulled, and made some edits (including "when passing/storing a reference to an unowned object" -> "when passing/storing an object's address" in order to avoid different meanings of "reference", cleaning up the example)

@chandlerc
Copy link
Contributor

I think the thread is coming to consensus now, and suggested an update to match what I think I'm seeing there... but also maybe I'm misreading so good to use the suggested change to make sure we're all on the same page.

@jonmeow jonmeow changed the title Brief style note on unowned objects Style notes on passing and storing object addresses Sep 20, 2024
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

LGTM, nice rewording too. =]

@chandlerc chandlerc added this pull request to the merge queue Sep 20, 2024
Merged via the queue into carbon-language:trunk with commit 434ee32 Sep 20, 2024
8 checks passed
@jonmeow jonmeow deleted the ptr-style branch September 23, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation An issue or proposed change to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants