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

document raw-addr-of operators #1001

Merged
merged 5 commits into from
Jul 17, 2022
Merged

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Apr 3, 2021

as requested by @Havvy

This is a bit unusual for being exposed as a library feature, fut it really is a fundamental language feature without stable syntax, which is stably accessible through the standard library.

Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

Just woke up and saw this, and it was a great way to start my day. 😍

I have to go to work, so no time for a full review, but I will at least ask that you switch the style to one line/one sentence to match the rest of the page.

@RalfJung
Copy link
Member Author

RalfJung commented Apr 4, 2021

I will at least ask that you switch the style to one line/one sentence to match the rest of the page.

Done.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 4, 2021
ptr::addr_of documentation improvements

While writing rust-lang/reference#1001 I figured I could also improve the docs here a bit.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 4, 2021
ptr::addr_of documentation improvements

While writing rust-lang/reference#1001 I figured I could also improve the docs here a bit.
src/expressions/operator-expr.md Outdated Show resolved Hide resolved
src/expressions/operator-expr.md Outdated Show resolved Hide resolved
### Raw address-of operators

Related to the borrow operators are the raw address-of operators, which do not have first-class syntax, but are exposed via the macros `ptr::addr_of!(expr)` and `ptr::addr_of_mut!(expr)`.
Like with `&`/`&mut`, the expression is evaluated in place expression context.
Copy link
Contributor

Choose a reason for hiding this comment

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

"the expression" here refers to the expression passed to the macro, right? If so, can you name that expression something; perhaps "the referenced operand". Also if so, it needs to be added to the list of place expression contexts on expressions.md.

Also, the compare/contrast with borrowing expressions feels weird in the context of the reference. It would make sense in material teaching for the first time, but this isn't a first time reader's document.

Copy link
Member Author

Choose a reason for hiding this comment

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

"the expression" here refers to the expression passed to the macro, right? If so, can you name that expression something; perhaps "the referenced operand". Also if so, it needs to be added to the list of place expression contexts on expressions.md.

I did some editing here, please check.

Also, the compare/contrast with borrowing expressions feels weird in the context of the reference. It would make sense in material teaching for the first time, but this isn't a first time reader's document.

I feel like this also makes sense just to increase clarity of the document, no matter how much experience we assume the reader to have. Also note that currently, ### Raw address-of operators is a third-level header, so this is basically "part of" the borrow operator section. The operators are very similar to I felt it would make sense to treat them together, at least to some extend.

src/expressions/operator-expr.md Outdated Show resolved Hide resolved
src/expressions/operator-expr.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

Overall I am happy with the changes. I just see one thing that really stands out to me.

src/expressions/operator-expr.md Outdated Show resolved Hide resolved
src/expressions/operator-expr.md Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member Author

@Havvy I think I did all the changes you were asking for.

@RalfJung
Copy link
Member Author

This PR has been waiting for the next round of review for a while (and the feature it documents is now stable since many releases). How can we make progress?

src/expressions/operator-expr.md Show resolved Hide resolved
src/expressions/operator-expr.md Outdated Show resolved Hide resolved
src/expressions/operator-expr.md Outdated Show resolved Hide resolved
@ehuss
Copy link
Contributor

ehuss commented Feb 23, 2022

Can you resolve the conflict?

This PR has been waiting for the next round of review for a while (and the feature it documents is now stable since many releases). How can we make progress?

I don't feel super comfortable hitting the merge button as I am not very familiar with this. I'd feel better if someone on the lang team approved it. Perhaps @joshtriplett?

@RalfJung
Copy link
Member Author

RalfJung commented Apr 4, 2022

@rust-lang/lang it's now been a year since the macros have been stabilized, might be good to also get the reference docs landed at some point. :D

@joshtriplett joshtriplett merged commit 5c4a7a6 into rust-lang:master Jul 17, 2022
@joshtriplett
Copy link
Member

I believe all feedback has been addressed at this point. Merging.

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.

4 participants