Skip to content

Pass CrateNum by value instead of by reference #82744

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Mar 4, 2021

It's more idiomatic to pass a small Copy type by value and CrateNum is
half the size of &CrateNum on 64-bit systems. The memory use change is
almost certainly insignificant, but why not!

It's more idiomatic to pass a small Copy type by value and `CrateNum` is
half the size of `&CrateNum` on 64-bit systems. The memory use change is
almost certainly insignificant, but why not!
@camelid camelid added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 4, 2021
@rust-highfive
Copy link
Contributor

r? @jyn514

(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 Mar 4, 2021
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Removing intermediate container 2e0debd11b85
 ---> 6126e6fa0ef4
Step 5/10 : RUN npm install es-check -g
 ---> Running in e1eb3e5a95af
/node-v14.4.0-linux-x64/bin/es-check -> /node-v14.4.0-linux-x64/lib/node_modules/es-check/index.js

> spawn-sync@1.0.15 postinstall /node-v14.4.0-linux-x64/lib/node_modules/es-check/node_modules/spawn-sync
> node postinstall
+ es-check@5.2.1
added 95 packages from 44 contributors in 4.804s
Removing intermediate container e1eb3e5a95af
 ---> 42f63225d1f5
---
Successfully built f4c504eff3fe
Successfully tagged rust-ci:latest
Built container sha256:f4c504eff3fe03938765fd3789cfff53c070b79df974416bee14ab4359974ebd
Uploading finished image to https://ci-caches.rust-lang.org/docker/9f2a38e35a8211f9c2c342213b6dabbf1ce1e957c3f9f4a6874af054aa93d446d1c6f8252277cb4118d76ddf7862eec7c972b385df9acf97d8b518b20c0181e6
upload failed: - to s3://rust-lang-ci-sccache2/docker/9f2a38e35a8211f9c2c342213b6dabbf1ce1e957c3f9f4a6874af054aa93d446d1c6f8252277cb4118d76ddf7862eec7c972b385df9acf97d8b518b20c0181e6 Unable to locate credentials
[CI_JOB_NAME=mingw-check]
---
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
Cloning into 'rust-toolstate'...
<Nothing changed>
+ es-check es5 ../src/librustdoc/html/static/main.js ../src/librustdoc/html/static/settings.js ../src/librustdoc/html/static/source-script.js ../src/librustdoc/html/static/storage.js

Cannot read property 'includes' of undefined

@camelid
Copy link
Member Author

camelid commented Mar 4, 2021

CI failure appears to be due to unrelated ESLint failure:

+ es-check es5 ../src/librustdoc/html/static/main.js ../src/librustdoc/html/static/settings.js ../src/librustdoc/html/static/source-script.js ../src/librustdoc/html/static/storage.js

Cannot read property 'includes' of undefined

cc @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

Same problem as in #82740

Did we change something recently in the CI?

@GuillaumeGomez
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 4, 2021

📌 Commit c79af86 has been approved by GuillaumeGomez

@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 Mar 4, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#80527 (Make rustdoc lints a tool lint instead of built-in)
 - rust-lang#82310 (Load rustdoc's JS search index on-demand.)
 - rust-lang#82315 (Improve page load performance in rustdoc)
 - rust-lang#82564 (Revert `Vec::spare_capacity_mut` impl to prevent pointers invalidation)
 - rust-lang#82697 (Fix stabilization version of move_ref_pattern)
 - rust-lang#82717 (Account for macros when suggesting adding lifetime)
 - rust-lang#82740 (Fix commit detected when using `download-rustc`)
 - rust-lang#82744 (Pass `CrateNum` by value instead of by reference)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 06630f7 into rust-lang:master Mar 4, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 4, 2021
@camelid camelid deleted the cratenum-byval branch March 5, 2021 22:30
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc 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