Skip to content

Rollup of 6 pull requests - #128672

Merged
bors merged 18 commits into
rust-lang:masterfrom
matthiaskrgr:rollup-txf7siy
Aug 5, 2024
Merged

Rollup of 6 pull requests#128672
bors merged 18 commits into
rust-lang:masterfrom
matthiaskrgr:rollup-txf7siy

Conversation

@matthiaskrgr

Copy link
Copy Markdown
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

RalfJungand others added 18 commits July 15, 2024 22:05
…eports
and remove the feature gate that silenced the lint
This allows the standard library to be compiled even with `download-rustc` enabled.
Which means it's no longer a requirement to compile `rustc` in order to compile `std`.
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
PR rust-lang#128581 introduced an assertion that all builtin attributes are
actually checked via `CheckAttrVisitor` and aren't accidentally usable
on completely unrelated HIR nodes. Unfortunately, the check had
correctness problems.
The match on attribute path segments looked like
```rust,ignore
[sym::should_panic] => /* check is implemented */
match BUILTIN_ATTRIBUTE_MAP.get(name) {
// checked below
Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
Some(_) => {
if !name.as_str().starts_with("rustc_") {
span_bug!(
attr.span,
"builtin attribute {name:?} not handled by `CheckAttrVisitor`"
)
}
}
None => (),
}
```
However, it failed to account for edge cases such as an attribute whose:
1. path segments *starts* with a builtin attribute such as
`should_panic`
2. which does not start with `rustc_`, and
3. is also an `AttributeType::Normal` attribute upon registration with
the builtin attribute map
These conditions when all satisfied cause the span bug to be issued for e.g.
`#[should_panic::skip]` because the `[sym::should_panic]` arm is not matched (since it's
`[sym::should_panic, sym::skip]`).
See <rust-lang#128622>.
…t, r=compiler-errors
turn `invalid_type_param_default` into a `FutureReleaseErrorReportInDeps`
`````@rust-lang/types````` I assume the plan is still to disallow this? It has been a future-compat lint for a long time, seems ripe to go for hard error.
However, turns out that outright removing it right now would lead to [tons of crater regressions](rust-lang#127655 (comment)), so for now this PR just makes this future-compat lint show up in cargo's reports, so people are warned when they use a dependency that is affected by this.
Fixesrust-lang#27336 by removing the feature gate (so there's no way to silence the lint even on nightly)
CC rust-lang#36887
…ct_with_derive, r=nnethercote
built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE hack and lint
Fixesrust-lang#107457 by turning the lint into a hard error. The lint has been shown in future breakage reports since Rust 1.69 (released in April 2023).
Let's see (via crater) if enough time has passed since rust-lang#104429, and unicode-org/icu4x#2834 has propagated far enough to let us make this a hard error.
Cc ``@nnethercote`` ``@Manishearth``
…k-Simulacrum
force compiling std from source if modified
This allows the standard library to be compiled even with `download-rustc` enabled. Which means it's no longer a requirement to compile `rustc` in order to compile `std`.
Ref. rust-lang#127322 (comment).
Blocker for rust-lang#122709.
Implement cursors for `BTreeSet`
Tracking issue: rust-lang#107540
This is a straightforward wrapping of the map API, except that map's `CursorMut` does not make sense, because there is no value to mutate. Hence, map's `CursorMutKey` is wrapped here as just `CursorMut`, since it's unambiguous for sets and we don't normally speak of "keys". On the other hand, I can see some potential for confusion with `CursorMut` meaning different things in each module. I'm happy to take suggestions to improve that.
r? ````@Amanieu````
…lacrum
Add test for updating enum discriminant through pointer
Closesrust-lang#122600
…cote
Do not fire unhandled attribute assertion on multi-segment `AttributeType::Normal` attributes with builtin attribute as first segment
### The Problem
In rust-lang#128581 I introduced an assertion to check that all builtin attributes are actually checked via
`CheckAttrVisitor` and aren't accidentally usable on completely unrelated HIR nodes.
Unfortunately, the assertion had correctness problems as revealed in rust-lang#128622.
The match on attribute path segments looked like
```rs,ignore
// Normal handler
[sym::should_panic] => /* check is implemented */
// Fallback handler
[name, ..] => match BUILTIN_ATTRIBUTE_MAP.get(name) {
// checked below
Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
Some(_) => {
if !name.as_str().starts_with("rustc_") {
span_bug!(
attr.span,
"builtin attribute {name:?} not handled by `CheckAttrVisitor`"
)
}
}
None => (),
}
```
However, it failed to account for edge cases such as an attribute whose:
1. path segments *starts* with a segment matching the name of a builtin attribute such as `should_panic`, and
2. the first segment's symbol does not start with `rustc_`, and
3. the matched builtin attribute is also of `AttributeType::Normal` attribute type upon registration with the builtin attribute map.
These conditions when all satisfied cause the span bug to be issued for e.g.
`#[should_panic::skip]` because the `[sym::should_panic]` arm is not matched (since it's
`[sym::should_panic, sym::skip]`).
### Proposed Solution
This PR tries to remedy that by adjusting all normal/specific handlers to not match exactly on a single segment, but instead match a prefix segment.
i.e.
```rs,ignore
// Normal handler, notice the `, ..` rest pattern
[sym::should_panic, ..] => /* check is implemented */
// Fallback handler
[name, ..] => match BUILTIN_ATTRIBUTE_MAP.get(name) {
// checked below
Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
Some(_) => {
if !name.as_str().starts_with("rustc_") {
span_bug!(
attr.span,
"builtin attribute {name:?} not handled by `CheckAttrVisitor`"
)
}
}
None => (),
}
```
### Review Remarks
This PR contains 2 commits:
1. The first commit adds a regression test. This will ICE without the `CheckAttrVisitor` changes.
2. The second commit adjusts `CheckAttrVisitor` assertion logic. Once this commit is applied, the test should no longer ICE and produce the expected bless stderr.
Fixesrust-lang#128622.
r? ``@nnethercote`` (since you reviewed rust-lang#128581)
@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 5, 2024
@matthiaskrgr

Copy link
Copy Markdown
MemberAuthor

@bors r+ rollup=never p=6

@bors

bors commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator

📌 Commit 2048007 has been approved by matthiaskrgr

It is now in the queue for this repository.

@borsbors 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 Aug 5, 2024
@bors

bors commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2048007 with merge 29e9248...

@bors

bors commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 29e9248 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Aug 5, 2024
@bors
bors merged commit 29e9248 into rust-lang:masterAug 5, 2024
@rustbotrustbot added this to the 1.82.0 milestone Aug 5, 2024
@rust-timer

Copy link
Copy Markdown
Collaborator

📌 Perf builds for each rolled up PR:

PR#MessagePerf Build Sha
#127655turn invalid_type_param_default into a `FutureReleaseErro…e2169ee6aa87e668dfa6d7e2f98eb55f19de0784 (link)
#127907built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DE…bcfb5101253e3e48d938edc7f9ee3e10b3a74448 (link)
#127974force compiling std from source if modified20eb7ac8a787c112a1c145662554649393fd7450 (link)
#128309Implement cursors for BTreeSetc20deadb4c787e9956412c54df53ff4c4f07fd27 (link)
#128500Add test for updating enum discriminant through pointer072fa398adc8cfc32181d90d5e8eb37bddd5b427 (link)
#128623Do not fire unhandled attribute assertion on multi-segment …39e577fb3e5bf4495700bcdd8ebeb6c6419037ef (link)

previous master: 176e545209

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (29e9248): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)
0.2%[0.2%, 0.2%]1
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
--0
All ❌✅ (primary)0.2%[0.2%, 0.2%]1

Max RSS (memory usage)

Results (primary -3.1%, secondary 0.9%)

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.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
0.9%[0.9%, 0.9%]1
Improvements ✅
(primary)
-3.1%[-3.1%, -3.1%]1
Improvements ✅
(secondary)
--0
All ❌✅ (primary)-3.1%[-3.1%, -3.1%]1

Cycles

Results (secondary 2.2%)

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.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
2.2%[2.2%, 2.2%]1
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
--0
All ❌✅ (primary)--0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 759.324s -> 759.685s (0.05%)
Artifact size: 336.79 MiB -> 336.80 MiB (0.00%)

@matthiaskrgr
matthiaskrgr deleted the rollup-txf7siy branch September 1, 2024 17:36
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-borsThis PR was explicitly merged by bors.rollupA PR which is a rollupS-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@matthiaskrgr@bors@rust-timer@rustbot@RalfJung@kmicklas@onur-ozkan@clubby789@jieyouxu