Closed
Conversation
…r_size` feature on some targets
Massage the `symbols` helpers to fill out {match all, match any} x
{substring match, exact match}:
| | Substring match | Exact match |
|-----------|----------------------------------------|-------------------------------|
| Match any | `object_contains_any_symbol_substring` | `object_contains_any_symbol` |
| Match all | `object_contains_all_symbol_substring` | `object_contains_all_symbols` |
As part of this, rename `any_symbol_contains` to
`object_contains_any_symbol_substring` for accuracy.
This PR adds `#[inline]` to the method `str::split_at_unchecked()`. This is done for two reasons: 1. The method is tiny, e.g. on AMD-64 (<https://godbolt.org/z/ba68fdfxn>): ```asm movq %rdi, %rax subq %rcx, %rdx movq %rsi, (%rdi) addq %rcx, %rsi movq %rcx, 8(%rdi) movq %rsi, 16(%rdi) movq %rdx, 24(%rdi) retq ``` 2. More importantly, inlining the method enables further automatic optimizations. E.g. if you split at index 3, then in the compiler (rustc, llvm or both) knows that this code cannot fail, and the panicking path is omitted in the generated code: ```rust pub fn punctuation(i: &str) -> Result<(), ()> { const THREE_CHARS: &[[u8; 3]] = &[*b"<<=", *b">>=", *b"...", *b"..="]; if let Some((head, _)) = i.split_at_checked(3) && THREE_CHARS.contains(&head.as_bytes().try_into().unwrap()) { Ok(()) } else { Err(()) } } ``` <details> <summary>Without PR</summary> <https://play.rust-lang.org/?version=stable&mode=release&edition=2024&gist=0234de8158f467eebd73286f20d6e27a> ```asm playground::punctuation: subq $40, %rsp movq %rsi, %rdx movq %rdi, %rsi movb $1, %al cmpq $3, %rdx ja .LBB2_2 je .LBB2_3 .LBB2_11: addq $40, %rsp retq .LBB2_2: cmpb $-64, 3(%rsi) jl .LBB2_11 .LBB2_3: leaq 8(%rsp), %rdi movl $3, %ecx callq *core::str::<impl str>::split_at_unchecked@GOTPCREL(%rip) movq 8(%rsp), %rcx movb $1, %al testq %rcx, %rcx je .LBB2_11 cmpq $3, 16(%rsp) jne .LBB2_12 movzwl (%rcx), %edx movzbl 2(%rcx), %ecx shll $16, %ecx orl %edx, %ecx cmpl $4013115, %ecx jg .LBB2_8 cmpl $3026478, %ecx je .LBB2_10 cmpl $4009518, %ecx je .LBB2_10 jmp .LBB2_11 .LBB2_8: cmpl $4013630, %ecx je .LBB2_10 cmpl $4013116, %ecx jne .LBB2_11 .LBB2_10: xorl %eax, %eax addq $40, %rsp retq .LBB2_12: leaq .Lanon.d98a7fbb86d10a97c24516e267466134.2(%rip), %rdi leaq .Lanon.d98a7fbb86d10a97c24516e267466134.1(%rip), %rcx leaq .Lanon.d98a7fbb86d10a97c24516e267466134.6(%rip), %r8 leaq 7(%rsp), %rdx movl $43, %esi callq *core::result::unwrap_failed@GOTPCREL(%rip) ``` </details> <details> <summary>With PR</summary> <https://play.rust-lang.org/?version=stable&mode=release&edition=2024&gist=5d4058c79ce0f6cb1a434190427d2055> ```asm playground::punctuation: movb $1, %al cmpq $3, %rsi ja .LBB0_2 je .LBB0_3 .LBB0_9: retq .LBB0_2: cmpb $-64, 3(%rdi) jl .LBB0_9 .LBB0_3: movzwl (%rdi), %eax movzbl 2(%rdi), %ecx shll $16, %ecx orl %eax, %ecx movb $1, %al cmpl $4013115, %ecx jg .LBB0_6 cmpl $3026478, %ecx je .LBB0_8 cmpl $4009518, %ecx je .LBB0_8 jmp .LBB0_9 .LBB0_6: cmpl $4013630, %ecx je .LBB0_8 cmpl $4013116, %ecx jne .LBB0_9 .LBB0_8: xorl %eax, %eax retq ``` </details>
having it on the impl block is a bit weird imo
musl's dlopen returns a different error than glibc, which contains the name of the file. This would cause the test to fail, since the filename would appear twice in the output (once in the error from rustc, once in the error message from musl). Split the expected test outputs for the different libc implementations. Signed-off-by: Jens Reidel <[email protected]>
This is quite a bit of implementation complexity, yet it is quite broken, and we don't have the maintenance bandwidth to address. Remove the current implementation if only to reduce bootstrap's implementation complexity; the `suggest` flow comes with its own set of hacks.
Implements https://www.github.com/rust-lang/rust/issues/141358. This has 2 primary benefits: 1. For rustdoc-json consumers, they no longer need to parse strings of attributes, but it's there in a structured and normalized way. 2. For rustc contributors, the output of HIR pretty printing is no longer a versioned thing in the output. People can work on rust-lang#131229 without needing to bump `FORMAT_VERSION`. (Over time, as the attribute refractor continues, I expect we'll add new things to `rustdoc_json_types::Attribute`. But this can be done separately to the rustc changes).
They are not always needed when building std, as is the case when packaging on Fedora. Panic if building from CI, but warn otherwise.
and make internal terminology consistent Co-authored-by: Travis Cross <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
This was referenced Jul 15, 2025
Collaborator
bors
added a commit
that referenced
this pull request
Jul 15, 2025
Rollup of 16 pull requests Successful merges: - #142301 (tests: Fix duplicated-path-in-error fail with musl) - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143630 (Drop `./x suggest`) - #143738 (Move several float tests to floats/mod.rs) - #143752 (Don't panic if WASI_SDK_PATH not set when detecting compiler) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143837 (Adjust `run_make_support::symbols` helpers) - #143878 (Port `#[pointee]` to the new attribute parsing infrastructure) - #143907 (core: make `str::split_at_unchecked()` inline) - #143910 (Add experimental `backtrace-trace-only` std feature) - #143927 (Preserve constness in trait objects up to hir ty lowering) - #143935 (rustc_type_ir/walk: move docstring to `TypeWalker` itself) - #143938 (Update books) - #143941 (update `cfg_select!` documentation) - #143948 (Update mdbook to 0.4.52) Failed merges: - #143926 (Remove deprecated fields in bootstrap) r? `@ghost` `@rustbot` modify labels: rollup
Member
|
@bors retry r- |
bors
added
S-waiting-on-author
Member
|
@bors r=jhpratt |
Collaborator
Collaborator
|
🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened. |
bors
added
S-waiting-on-bors
Member
|
@/bors treeclosed- |
Collaborator
bors
added a commit
that referenced
this pull request
Jul 15, 2025
Rollup of 16 pull requests Successful merges: - #142301 (tests: Fix duplicated-path-in-error fail with musl) - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143630 (Drop `./x suggest`) - #143738 (Move several float tests to floats/mod.rs) - #143752 (Don't panic if WASI_SDK_PATH not set when detecting compiler) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143837 (Adjust `run_make_support::symbols` helpers) - #143878 (Port `#[pointee]` to the new attribute parsing infrastructure) - #143907 (core: make `str::split_at_unchecked()` inline) - #143910 (Add experimental `backtrace-trace-only` std feature) - #143927 (Preserve constness in trait objects up to hir ty lowering) - #143935 (rustc_type_ir/walk: move docstring to `TypeWalker` itself) - #143938 (Update books) - #143941 (update `cfg_select!` documentation) - #143948 (Update mdbook to 0.4.52) Failed merges: - #143926 (Remove deprecated fields in bootstrap) r? `@ghost` `@rustbot` modify labels: rollup
Collaborator
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
Collaborator
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
Member
Huh? |
Contributor
|
This is because https://github.com/rust-lang/rust/pull/142936/files adds a file with |
Member
|
Ah, cheers. |
4 tasks
Member
bors
added
S-waiting-on-author
rustbot
removed
the
S-waiting-on-author
Collaborator
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
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:
./x suggest#143630 (Drop./x suggest)optimize_for_sizefeature on some targets #143820 (Fixed a core crate compilation failure when enabling theoptimize_for_sizefeature on some targets)run_make_support::symbolshelpers #143837 (Adjustrun_make_support::symbolshelpers)#[pointee]to the new attribute parsing infrastructure #143878 (Port#[pointee]to the new attribute parsing infrastructure)str::split_at_unchecked()inline #143907 (core: makestr::split_at_unchecked()inline)backtrace-trace-onlystd feature #143910 (Add experimentalbacktrace-trace-onlystd feature)TypeWalkeritself #143935 (rustc_type_ir/walk: move docstring toTypeWalkeritself)cfg_select!documentation #143941 (updatecfg_select!documentation)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup