Rollup of 7 pull requests #95855
Merged
Rollup of 7 pull requests #95855
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The problem was two-fold: - Bootstrap was hard-coding that unit tests should always run with stage1, not stage2, and - It hard-coded the sysroot layout in stage1, which puts libLLVM.so in `lib/rustlib/` instead of just `lib/`.
Previously, it would erroneously try to run the doc-tests anyway and give an error: ``` Doc-tests rustdoc thread 'main' panicked at 'RUSTDOC_LIBDIR was not set', src/bootstrap/bin/rustdoc.rs:15:48 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace error: test failed, to rerun pass '--doc' ```
This is a repeat for Rc of e0e64a8, which cleaned up the same thing for Arc.
This saves an instruction compared to the previous approach, which was to unset the fifth bit with bitwise OR.
Inspired by the zulip conversation about how `Layout` should better enforce `size < isize::MAX as usize`, this uses an N-variant enum on N-bit platforms to require at the validity level that the existing invariant of "must be a power of two" is upheld. This was MIRI can catch it, and means there's a more-specific type for `Layout` to store than just `NonZeroUsize`.
It was used for deduplicating some errors for legacy code which are mostly deduplicated even without that, but at cost of global mutable state, which is not a good tradeoff.
**This Commit** Adds some clarity around indexing into Strings and the constraints driving various decisions there. **Why?** The [`String` documentation][0] mentions how `String`s can't be indexed but `Range` has an implementation for `SliceIndex<str>`. This can be confusing. There are also several statements to explain the lack of `String` indexing: - the inability to index into a `String` is an implication of UTF-8 encoding - indexing into a `String` could not be constant-time with UTF-8 encoding - indexing into a `String` does not have an obvious return type This last statement made sense but the first two seemed contradictory to the documentation around [`SliceIndex<str>`][1] which mention: - one can index into a `String` with a `Range` (also called substring slicing but it uses the same syntax and the method name is `index`) - `Range` indexing into a `String` is constant-time To resolve this seeming contradiction the documentation is reworked to more clearly explain what factors drive the decision to disallow indexing into a `String` with a single number. [0]: https://doc.rust-lang.org/stable/std/string/struct.String.html#utf-8 [1]: https://doc.rust-lang.org/stable/std/slice/trait.SliceIndex.html#impl-SliceIndex%3Cstr%3E
…g-indexing-docs, r=Mark-Simulacrum Clarify indexing into Strings **This Commit** Adds some clarity around indexing into Strings. **Why?** I was reading through the `Range` documentation and saw an implementation for `SliceIndex<str>`. I was surprised to see this and went to read the [`String`][0] documentation and, to me, it seemed to say (at least) three things: 1. you cannot index into a `String` 2. indexing into a `String` could not be constant-time 3. indexing into a `String` does not have an obvious return type I absolutely agree with the last point but the first two seemed contradictory to the documentation around [`SliceIndex<str>`][1] which mention: 1. you can do substring slicing (which is probably different than "indexing" but, because the method is called `index` and I associate anything with square brackets with "indexing" it was enough to confuse me) 2. substring slicing is constant-time (this may be algorithmic ignorance on my part but if `&s[i..i+1]` is O(1) then it seems confusing that `&s[i]` _could not possibly_ be O(1)) So I was hoping to clarify a couple things and, hopefully, in this PR review learn a little more about the nuances here that confused me in the first place. [0]: https://doc.rust-lang.org/stable/std/string/struct.String.html#utf-8 [1]: https://doc.rust-lang.org/stable/std/slice/trait.SliceIndex.html#impl-SliceIndex%3Cstr%3E
…crum Make non-power-of-two alignments a validity error in `Layout` Inspired by the zulip conversation about how `Layout` should better enforce `size <= isize::MAX as usize`, this uses an N-variant enum on N-bit platforms to require at the validity level that the existing invariant of "must be a power of two" is upheld. This was MIRI can catch it, and means there's a more-specific type for `Layout` to store than just `NonZeroUsize`. It's left as `pub(crate)` here; a future PR could consider giving it a tracking issue for non-internal usage.
Fix `x test src/librustdoc` with `download-rustc` enabled The problem was two-fold: - Bootstrap was hard-coding that unit tests should always run with stage1, not stage2, and - It hard-coded the sysroot layout in stage1, which puts libLLVM.so in `lib/rustlib/` instead of just `lib/`. This also takes the liberty of fixing `test src/librustdoc --no-doc`, which has been broken since it was first added. It would be nice at some point to unify this logic with other tests; I opened a Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Inconsistency.20in.20.60x.20test.60 Fixes rust-lang#91071.
Left overs of rust-lang#95761 These are just nits. Feel free to close this PR if all modifications are not worth merging. * `#![feature(decl_macro)]` is not needed anymore in `rustc_expand` * `tuple_impls` does not require `$Tuple:ident`. I guess it is there to enhance readability? r? ```@petrochenkov```
expand: Remove `ParseSess::missing_fragment_specifiers` It was used for deduplicating some errors for legacy code which are mostly deduplicated even without that, but at cost of global mutable state, which is not a good tradeoff. cc rust-lang#95747 (comment) r? ``@nnethercote``
hide another #[allow] directive from a docs example This is a repeat for Rc of e0e64a8, which cleaned up the same thing for Arc.
Use bitwise XOR in to_ascii_uppercase This saves an instruction compared to the previous approach, which was to unset the fifth bit with bitwise OR. Comparison of generated assembly on x86: https://godbolt.org/z/GdfvdGs39 This can also affect autovectorization, saving SIMD instructions as well: https://godbolt.org/z/cnPcz75T9 Not sure if `u8::to_ascii_lowercase` should also be changed, since using bitwise OR for that function does not require an extra bitwise negate since the code is setting a bit rather than unsetting a bit. `char::to_ascii_uppercase` already uses XOR, so no change seems to be required there.
rustbot
added
T-compiler
|
@bors r+ rollup=never p=5 |
|
📌 Commit 7726265 has been approved by |
bors
added
the
S-waiting-on-bors
|
⌛ Testing commit 7726265 with merge 86a62f25f94b49d8a8375dacf0a90f19cfa57899... |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@bors retry |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (8bf93e9): comparison url. Summary:
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
Layout#95361 (Make non-power-of-two alignments a validity error inLayout)x test src/librustdocwithdownload-rustcenabled #95369 (Fixx test src/librustdocwithdownload-rustcenabled )macro_metavar_expr#95761)ParseSess::missing_fragment_specifiers#95808 (expand: RemoveParseSess::missing_fragment_specifiers)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup