Skip to content

Conversation

kornelski
Copy link
Contributor

I've added #[track_caller] to String methods that delegate to Vec methods that already have #[track_caller].

I've also added #[track_caller] to methods that have assert! or panic! due to invalid inputs.

@rustbot
Copy link
Collaborator

rustbot commented Jun 19, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 19, 2025
@tgross35
Copy link
Contributor

Since this sometimes increases compile times

@bors2 try
@rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Jun 19, 2025

⌛ Trying commit c109b28 with merge 075cf9a

To cancel the try build, run the command @bors2 try cancel.

rust-bors bot added a commit that referenced this pull request Jun 19, 2025
Let String pass #[track_caller] to its Vec calls

I've added `#[track_caller]` to `String` methods that delegate to `Vec` methods that already have `#[track_caller]`.

I've also added `#[track_caller]` to methods that have `assert!` or `panic!` due to invalid inputs.
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 19, 2025
@rust-bors
Copy link

rust-bors bot commented Jun 19, 2025

☀️ Try build successful (CI)
Build commit: 075cf9a (075cf9a3e6ff4d06acf01becd0b8e06b926329e1, parent: 2fcf1776b9ccef89993dfe40e9f5c4908e2d2d48)

@rust-timer

This comment has been minimized.

@tgross35
Copy link
Contributor

Mind posting a difference in output with and without this change for reference?

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (075cf9a): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.4% [-0.4%, -0.4%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.0% [-2.0%, -2.0%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [-2.0%, 2.8%] 2

Cycles

Results (secondary 4.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.6% [4.6%, 4.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

Results (primary 0.1%, secondary 0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.3%] 12
Regressions ❌
(secondary)
0.3% [0.0%, 0.4%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [0.0%, 0.3%] 12

Bootstrap: 692.244s -> 692.328s (0.01%)
Artifact size: 371.99 MiB -> 372.07 MiB (0.02%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 20, 2025
@kornelski
Copy link
Contributor Author

old:

thread 'main' panicked at ~/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/string.rs:1461:13:
assertion failed: self.is_char_boundary(new_len)

new:

thread 'main' panicked at src/main.rs:8:7:
assertion failed: self.is_char_boundary(new_len)

@tgross35
Copy link
Contributor

Perf looks okay and the message is indeed much better, thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 20, 2025

📌 Commit c109b28 has been approved by tgross35

It is now in the queue for this repository.

@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 Jun 20, 2025
@bors
Copy link
Collaborator

bors commented Jun 22, 2025

⌛ Testing commit c109b28 with merge 8387d61...

@bors
Copy link
Collaborator

bors commented Jun 23, 2025

☀️ Test successful - checks-actions
Approved by: tgross35
Pushing 8387d61 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 23, 2025
@bors bors merged commit 8387d61 into rust-lang:master Jun 23, 2025
11 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 23, 2025
Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing be19eda (parent) -> 8387d61 (this PR)

Test differences

Show 104 test diffs

104 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 8387d61a6ec05faee58f7cfb9c10799285067934 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-apple-1: 11062.1s -> 6726.2s (-39.2%)
  2. dist-x86_64-apple: 8900.1s -> 8198.2s (-7.9%)
  3. mingw-check-2: 1900.2s -> 2039.3s (7.3%)
  4. mingw-check-tidy: 77.9s -> 72.3s (-7.3%)
  5. dist-ohos-armv7: 4194.0s -> 3892.8s (-7.2%)
  6. x86_64-apple-2: 3566.7s -> 3312.0s (-7.1%)
  7. x86_64-gnu-llvm-19-2: 6050.4s -> 5663.6s (-6.4%)
  8. arm-android: 5463.5s -> 5164.8s (-5.5%)
  9. test-various: 4493.4s -> 4250.3s (-5.4%)
  10. x86_64-msvc-2: 6968.8s -> 6615.7s (-5.1%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8387d61): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.9% [-2.9%, -2.9%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.9% [-2.9%, -2.9%] 1

Max RSS (memory usage)

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

Cycles

Results (primary -2.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.4% [-2.4%, -2.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.4% [-2.4%, -2.4%] 1

Binary size

Results (primary -0.0%, secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.3%] 11
Regressions ❌
(secondary)
0.2% [0.0%, 0.3%] 2
Improvements ✅
(primary)
-1.1% [-1.1%, -1.1%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.0% [-1.1%, 0.3%] 12

Bootstrap: 689.346s -> 689.715s (0.05%)
Artifact size: 371.89 MiB -> 371.92 MiB (0.01%)

@kornelski kornelski deleted the string-track branch June 23, 2025 16:37
tautschnig pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 3, 2025
Let String pass #[track_caller] to its Vec calls

I've added `#[track_caller]` to `String` methods that delegate to `Vec` methods that already have `#[track_caller]`.

I've also added `#[track_caller]` to methods that have `assert!` or `panic!` due to invalid inputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant 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.

5 participants