Rollup of 9 pull requests #128253
Merged
Rollup of 9 pull requests #128253
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 current title is too similar to that of the page for std::result::Result, which is a problem both for navigating to the Result docs via browser autocomplete, and for being able to tell which tab is which when the width of tabs is small.
This has been bugging me for a while. I find complex "if any of these are true" conditions easier to think about than complex "if all of these are true" conditions, because you can stop as soon as one is true.
It has a single call site. This change makes the two `needs_collect` conditions more similar to each other, and therefore easier to understand.
And make it non-`pub`.
I have always found `is_complete` an unhelpful name. The new name (and inverted sense) fits in better with the conditions at its call sites.
The current code is this: ``` self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target))); self.capture_state.replace_ranges.extend(inner_attr_replace_ranges); ``` What's not obvious is that every range in `inner_attr_replace_ranges` must be a strict sub-range of `start_pos..end_pos`. Which means, in `LazyAttrTokenStreamImpl::to_attr_token_stream`, they will be done first, and then the `start_pos..end_pos` replacement will just overwrite them. So they aren't needed.
Imagine you have replace ranges (2..20,X) and (5..15,Y), and these tokens: ``` a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x ``` If we replace (5..15,Y) first, then (2..20,X) we get this sequence ``` a,b,c,d,e,Y,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x ``` which is what we want. If we do it in the other order, we get this: ``` a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x a,b,X,_,_,Y,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x ``` which is wrong. So it's true that we need the `.rev()` but the comment is wrong about why.
A fully imperative style is easier to read than a half-iterator, half-imperative style. Also, rename `inner_attr` as `attr` because it might be an outer attribute.
This was handled correctly already for `extern unsafe fn()`. Co-authored-by: Folkert <folkert@folkertdev.nl>
…tr, r=dtolnay
Stabilize const `{integer}::from_str_radix` i.e. `const_int_from_str`
This PR stabilizes the feature `const_int_from_str`.
- ACP Issue: rust-lang/libs-team#74
- Implementation PR: rust-lang#99322
- Part of Tracking Issue: rust-lang#59133
API Change Diff:
```diff
impl {integer} {
- pub fn from_str_radix(src: &str, radix: u32) -> Result<Self, ParseIntError>;
+ pub const fn from_str_radix(src: &str, radix: u32) -> Result<Self, ParseIntError>;
}
impl ParseIntError {
- pub fn kind(&self) -> &IntErrorKind;
+ pub const fn kind(&self) -> &IntErrorKind;
}
```
This makes it easier to parse integers at compile-time, e.g.
the example from the Tracking Issue:
```rust
env!("SOMETHING").parse::<usize>().unwrap()
```
could now be achived with
```rust
match usize::from_str_radix(env!("SOMETHING"), 10) {
Ok(val) => val,
Err(err) => panic!("Invalid value for SOMETHING environment variable."),
}
```
rather than having to depend on a library that implements or manually implement the parsing at compile-time.
---
Checklist based on [Libs Stabilization Guide - When there's const involved](https://std-dev-guide.rust-lang.org/development/stabilization.html#when-theres-const-involved)
I am treating this as a [partial stabilization](https://std-dev-guide.rust-lang.org/development/stabilization.html#partial-stabilizations) as it shares a tracking issue (and is rather small), so directly opening the partial stabilization PR for the subset (feature `const_int_from_str`) being stabilized.
- [x] ping Constant Evaluation WG
- [x] no unsafe involved
- [x] no `#[allow_internal_unstable]`
- [ ] usage of `intrinsic::const_eval_select` rust-lang#124625 in `from_str_radix_assert` to change the error message between compile-time and run-time
- [ ] [rust-labg/libs-api FCP](rust-lang#124941 (comment))
…li-obk Implement `Copy`/`Clone` for async closures We can do so in the same cases that regular closures do. For the purposes of cloning, coroutine-closures are actually precisely the same as regular closures, specifically in the aspect that `Clone` impls care about which is the upvars. The only difference b/w coroutine-closures and regular closures is the type that they *return*, but this type has not been *created* yet, so we don't really have a problem. IDK why I didn't add this impl initially -- I went back and forth a bit on the internal representation for coroutine-closures before settling on a design which largely models regular closures. Previous (not published) iterations of coroutine-closures used to be represented as a special (read: cursed) kind of coroutine, which would probably suffer from the pitfalls that coroutines have that oli mentioned below in rust-lang#128201 (comment). r? oli-obk
… r=notriddle,GuillaumeGomez rustdoc: change title of search results the current title is too similar to that of the page for std::result::Result, which is a problem both for navigating to the Result docs via browser autocomplete, and for being able to tell which tab is which when the width of tabs is small.
…s, r=petrochenkov Refactor complex conditions in `collect_tokens_trailing_token` More readability improvements for this complicated function. r? ````@petrochenkov````
…r=petrochenkov Remove unnecessary range replacements This PR removes an unnecessary range replacement in `collect_tokens_trailing_token`, and does a couple of other small cleanups. r? ````@petrochenkov````
…etrochenkov Remove redundant option that was just encoding that a slice was empty There is already a sanity check ensuring we don't put empty attribute lists into the HIR: https://github.com/rust-lang/rust/blob/6ef11b81c2c02c3c4b7556d1991a98572fe9af87/compiler/rustc_ast_lowering/src/lib.rs#L661-L667
…ix, r=tgross35 CI: do not respect custom try jobs for unrolled perf builds Before this PR, if a pull request merged in a rollup had some `try-job` annotations, the unrolled perf builds were running the custom try jobs instead of the default job, which was wrong. Found out [here](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/try-perf.20jobs.20respect.20try-job.20annotations).
…compiler-errors Improve `extern "<abi>" unsafe fn()` error message These errors were already reported in rust-lang#87217, and fixed by rust-lang#87235 but missed the case of an explicit ABI. This PR does not cover multiple keywords like `extern "C" pub const unsafe fn()`, but I don't know what a good way to cover this would be. It also seems rarer than `extern "C" unsafe` which I saw happen a few times in workshops.
…ocs, r=tgross35 Fix `Iterator::filter` docs Small fix to add code formatting around `Iterator::filter` `true` return type
rustbot
added
A-testsuite
|
@bors r+ rollup=never p=9 |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR:
previous master: 7c2012d0ec In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
This was referenced Jul 27, 2024
|
Finished benchmarking commit (8b6b857): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (primary -1.1%, secondary -1.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -1.9%, secondary -3.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 770.229s -> 770.348s (0.02%) |
|
visiting for weekly rustc-perf triage seems like noise from the graph over time. @rustbot label: +perf-regression-triaged |
rustbot
added
the
perf-regression-triaged
Folyd
added a commit
to Folyd/rust
that referenced
this pull request
Aug 21, 2024
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:
{integer}::from_str_radixi.e.const_int_from_str#124941 (Stabilize const{integer}::from_str_radixi.e.const_int_from_str)Copy/Clonefor async closures #128201 (ImplementCopy/Clonefor async closures)collect_tokens_trailing_token#128223 (Refactor complex conditions incollect_tokens_trailing_token)extern "<abi>" unsafe fn()error message #128229 (Improveextern "<abi>" unsafe fn()error message)Iterator::filterdocs #128235 (FixIterator::filterdocs)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup