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

Comments in libsyntax/ptr.rs contain outdated references to @T #67341

Closed
alex opened this issue Dec 16, 2019 · 2 comments · Fixed by #67394
Closed

Comments in libsyntax/ptr.rs contain outdated references to @T #67341

alex opened this issue Dec 16, 2019 · 2 comments · Fixed by #67394
Assignees
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@alex
Copy link
Member

alex commented Dec 16, 2019

//! The AST pointer.
//!
//! Provides `P<T>`, a frozen owned smart pointer, as a replacement for `@T` in
//! the AST.
//!
//! # Motivations and benefits
//!
//! * **Identity**: sharing AST nodes is problematic for the various analysis
//! passes (e.g., one may be able to bypass the borrow checker with a shared
//! `ExprKind::AddrOf` node taking a mutable borrow). The only reason `@T` in the
//! AST hasn't caused issues is because of inefficient folding passes which
//! would always deduplicate any such shared nodes. Even if the AST were to
//! switch to an arena, this would still hold, i.e., it couldn't use `&'a T`,
//! but rather a wrapper like `P<'a, T>`.
//!
//! * **Immutability**: `P<T>` disallows mutating its inner `T`, unlike `Box<T>`
//! (unless it contains an `Unsafe` interior, but that may be denied later).
//! This mainly prevents mistakes, but can also enforces a kind of "purity".
//!
//! * **Efficiency**: folding can reuse allocation space for `P<T>` and `Vec<T>`,
//! the latter even when the input and output types differ (as it would be the
//! case with arenas or a GADT AST using type parameters to toggle features).
//!
//! * **Maintainability**: `P<T>` provides a fixed interface - `Deref`,
//! `and_then` and `map` - which can remain fully functional even if the
//! implementation changes (using a special thread-local heap, for example).
//! Moreover, a switch to, e.g., `P<'a, T>` would be easy and mostly automated.

The comments are quite old and probably need some small updates to reflect @T having been removed from the language.

This issue has been assigned to @matthew-healy via this comment.

@csmoe csmoe added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Dec 16, 2019
@matthew-healy
Copy link
Contributor

I think I should be able to handle this. Hopefully some time in the next few days.

Am I right in thinking I just do this? @rustbot claim

I've found the documentation style RFC, and some info about what @T was, which should hopefully be enough info to improve this comment. 👍 If there's anything else specific I should consider plz let me know.

@rustbot rustbot self-assigned this Dec 17, 2019
@Mark-Simulacrum
Copy link
Member

Yes, you can definitely just do this and any questions which come up can be addressed on the (eventual) PR most likely. I imagine it's mostly a matter of just deleting @T related documentation; if you want to try and replace it with something more up to date that would be even better.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Dec 19, 2019
…docs, r=Dylan-DPC

Remove outdated references to @t from comments

Closes rust-lang#67341.

This removes all references to `@T` from the comment in libsyntax/ptr.rs.
@bors bors closed this as completed in cfa7581 Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants