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

Arenas cleanup #90990

Merged
merged 2 commits into from
Nov 19, 2021
Merged

Arenas cleanup #90990

merged 2 commits into from
Nov 19, 2021

Conversation

nnethercote
Copy link
Contributor

I was looking closely at the arenas code and here are some small improvement to readability.

Because it's always `'tcx`. In fact, some of them use a mixture of
passed-in `$tcx` and hard-coded `'tcx`, so no other lifetime would even
work.

This makes the code easier to read.
@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 17, 2021
@nnethercote
Copy link
Contributor Author

@bors rollup=always

Because this is a combination of comment changes and trivial code changes.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 18, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Nov 18, 2021

📌 Commit e73784d has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 18, 2021
Arenas cleanup

I was looking closely at the arenas code and here are some small improvement to readability.
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 18, 2021
@JohnTitor
Copy link
Member

Failed in rollup: #91016 (comment)
@bors r-

 error: this URL is not a hyperlink
   --> compiler/rustc_arena/src/lib.rs:338:13
    |
338 |     /// see fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.>`
    |
    = note: `-D rustdoc::bare-urls` implied by `-D warnings`
    = note: bare URLs are not automatically turned into clickable links

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 18, 2021
Also use `Default::default()` in one `TypedArena::default()`, for
consistency with `DroplessArena::default()`.
@nnethercote
Copy link
Contributor Author

 error: this URL is not a hyperlink
   --> compiler/rustc_arena/src/lib.rs:338:13
    |
338 |     /// see fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<fitzgeraldnick.com/2019/11/01/always-bump-downwards.html.>`

Huh, I didn't see this with ./x.py test. Is there a command I could have run locally to catch this?

Also, curious that the leading https:// has been stripped in that error message.

Anyway, I've updated: @bors r=oli-obk

@bors
Copy link
Contributor

bors commented Nov 18, 2021

📌 Commit 0a89598 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2021
@JohnTitor
Copy link
Member

Huh, I didn't see this with ./x.py test. Is there a command I could have run locally to catch this?

The warning is generated by rustdoc so you could see it via ./x.py doc, I guess (yeah, it's kinda annoying to use that command just for a compiler/library change..., I don't know how we trigger it via other commands.)

Also, curious that the leading https:// has been stripped in that error message.

The scheme somehow has been stripped when copy/pasting 🤔, it's not a fault of the warning.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 18, 2021
Arenas cleanup

I was looking closely at the arenas code and here are some small improvement to readability.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#89258 (Make char conversion functions unstably const)
 - rust-lang#90578 (add const generics test)
 - rust-lang#90633 (Refactor single variant `Candidate` enum into a struct)
 - rust-lang#90800 (bootstap: create .cargo/config only if not present)
 - rust-lang#90942 (windows: Return the "Not Found" error when a path is empty)
 - rust-lang#90947 (Move some tests to more reasonable directories - 9.5)
 - rust-lang#90961 (Suggest removal of arguments for unit variant, not replacement)
 - rust-lang#90990 (Arenas cleanup)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1576a7c into rust-lang:master Nov 19, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 19, 2021
@nnethercote nnethercote deleted the arenas-cleanup branch November 19, 2021 09:40
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 27, 2022
…oli-obk

More arena cleanups

A sequel to rust-lang#90990.

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants