Add Location::file_with_nul by cramertj · Pull Request #135054 · rust-lang/rust

cramertj · 2025-01-03T01:53:45Z

This is useful for C/C++ APIs which expect the const char* returned from FILE or std::source_location::file_name.

ACP: rust-lang/libs-team#466
Tracking issue: #141727

rustbot · 2025-01-03T01:53:53Z

rustbot has assigned @cuviper.
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 · 2025-01-03T01:53:55Z

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Noratrieb · 2025-01-03T08:58:51Z

see rust-lang/libs-team#466

compiler/rustc_const_eval/src/interpret/place.rs

compiler/rustc_const_eval/src/util/caller_location.rs

library/core/src/panic/location.rs

cramertj · 2025-01-03T17:21:10Z

see rust-lang/libs-team#466

Renamed to file_with_nul as per that proposal.

bjorn3 · 2025-01-05T10:36:30Z

How does this compare with #131828?

cramertj · 2025-01-06T21:34:58Z

@bjorn3 This is essentially the same feature, although this implementation doesn't require an increase to the size of the Location struct itself (and could actually be used to decrease it by dropping the size of the slice if we cared to do so).

Darksonn · 2025-01-07T14:58:26Z

The other PR does not change the size of the Location struct either.

oli-obk · 2025-01-07T15:28:39Z

library/core/src/panic/location.rs

+            let str_len = self.file_bytes_with_nul.len() - 1;
+            // SAFETY: `file_bytes_with_nul` without the trailing nul byte is guaranteed to be
+            // valid UTF8.
+            unsafe { crate::str::from_raw_parts(self.file_bytes_with_nul.as_ptr(), str_len) }


str is allowed to have null bytes within. So you could keep the type at &'a str and use &self.file[..self.file.len() -1] here without unsafe code

I could! I think that just moves the unsafety into a call to from_bytes_with_nul_unchecked for the CStr, though, so I think it's roughly the same either way.

That unsafety exists either way unless you just directly encode it as CStr, tho we're still holding out for that becoming a thin pointer, at which point it would be expensive to get the str out of it.

Maybe it's simpler to just store a raw pointer and length without the nul-terminator included. That way, Location hold the integer we want most of the time without triggering provenance concerns.

That way, Location hold the integer we want most of the time without triggering provenance concerns.

What integer do you mean?

I wasn't aware of any provenance concerns, was that discussed in #131828?

The provenance discussion is in a hidden subthread above: #135054 (comment)

I mean that storing a raw pointer / length instead of a slice would let us store the length without the nul-terminator included, which makes more sense to me.

compiler/rustc_const_eval/src/util/caller_location.rs

cramertj · 2025-01-07T16:19:41Z

The other PR does not change the size of the Location struct either.

Thanks for the correction! I must've misread it when I saw it. In either case, I don't have a strong preference and am happy to remove this one if it makes life easier. I didn't know about the first PR when I wrote this one.

Darksonn · 2025-01-08T09:14:21Z

I closed my original PR in favor of this PR, and opened a new PR #135240 that shows how the alternative approach of adding a compiler flag would look.

Noratrieb · 2025-01-08T17:58:09Z

Let's do a perf run to get a concrete binary size overhead (which will of course be so tiny that it's negligible, but having proof of that seems to be necessary in this discussion!).
@bors try @rust-timer queue

bors · 2025-01-08T17:59:23Z

⌛ Trying commit 73810aa with merge 35c365c...

Add Location::file_with_nul This is useful for C/C++ APIs which expect the const char* returned from __FILE__ or std::source_location::file_name.

bors · 2025-01-08T19:50:53Z

☀️ Try build successful - checks-actions
Build commit: 35c365c (35c365c6fce43dba6f1e28794102c6f406ba02d9)

This is useful for C/C++ APIs which expect the const char* returned from __FILE__ or std::source_location::file_name.

m-ou-se · 2025-06-04T17:33:32Z

bors · 2025-06-04T17:33:35Z

📌 Commit b541f93 has been approved by m-ou-se

It is now in the queue for this repository.

Add Location::file_with_nul This is useful for C/C++ APIs which expect the const char* returned from __FILE__ or std::source_location::file_name. ACP: rust-lang/libs-team#466 Tracking issue: #141727

bors · 2025-06-05T03:53:42Z

⌛ Testing commit b541f93 with merge d1634d0...

rust-log-analyzer · 2025-06-05T05:25:52Z

The job dist-x86_64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[TIMING] core::build_steps::doc::TheBook { compiler: Compiler { stage: 2, host: x86_64-apple-darwin, forced_compiler: false }, target: x86_64-apple-darwin } -- 2.149
##[group]Documenting stage2 standalone (x86_64-apple-darwin)
##[endgroup]
[TIMING] core::build_steps::doc::Standalone { compiler: Compiler { stage: 2, host: x86_64-apple-darwin, forced_compiler: false }, target: x86_64-apple-darwin } -- 0.818
dyld[44097]: Library not loaded: @rpath/librustc_driver-0e8537a46b37ce74.dylib
  Referenced from: <0E2B9387-EEC1-3F9A-8FA4-7BED87576B3D> /Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/rustc-main
  Reason: tried: '/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/../lib/librustc_driver-0e8537a46b37ce74.dylib' (no such file), '/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-rustc/x86_64-apple-darwin/release/../lib/librustc_driver-0e8537a46b37ce74.dylib' (no such file)
Command "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage2/bin/rustc" "--target" "x86_64-apple-darwin" "--print=deployment-target" (failure_mode=Exit) has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 1:23:47
  local time: Thu Jun  5 05:25:39 UTC 2025
  network time: Thu, 05 Jun 2025 05:25:40 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

bors · 2025-06-05T05:26:28Z

💔 Test failed - checks-actions

m-ou-se · 2025-06-05T07:03:48Z

@bors retry

bors · 2025-06-05T10:21:23Z

⌛ Testing commit b541f93 with merge c360e21...

bors · 2025-06-05T13:43:35Z

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing c360e21 to master...

github-actions · 2025-06-05T13:46:42Z

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 425e142 (parent) -> c360e21 (this PR)

Test differences

Show 12 test diffs

Stage 1

[ui] tests/ui/rfcs/rfc-2091-track-caller/file-is-nul-terminated.rs: [missing] -> pass (J0)

Stage 2

[ui] tests/ui/rfcs/rfc-2091-track-caller/file-is-nul-terminated.rs: [missing] -> pass (J1)

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

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard c360e219f5a56631baa46065d28e9852ca7d4ce3 --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

dist-apple-various: 5703.5s -> 9876.7s (73.2%)
x86_64-apple-1: 5922.1s -> 7572.1s (27.9%)
dist-aarch64-apple: 5742.9s -> 6609.7s (15.1%)
x86_64-apple-2: 3552.1s -> 3130.4s (-11.9%)
dist-x86_64-apple: 8479.4s -> 7514.7s (-11.4%)
dist-armv7-linux: 5348.1s -> 4863.7s (-9.1%)
dist-loongarch64-musl: 4761.0s -> 5139.6s (8.0%)
aarch64-apple: 4342.9s -> 4005.1s (-7.8%)
dist-various-1: 4483.9s -> 4146.4s (-7.5%)
x86_64-msvc-2: 6435.7s -> 6891.8s (7.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 · 2025-06-05T16:26:45Z

Finished benchmarking commit (c360e21): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary -1.2%, secondary -3.3%)

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.

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

Cycles

Results (secondary 3.3%)

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.

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

Binary size

Results (primary -0.1%, secondary -0.1%)

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.

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	3
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-0.1%	[-0.3%, -0.0%]	82
Improvements ✅ (secondary)	-0.1%	[-1.7%, -0.0%]	61
All ❌✅ (primary)	-0.1%	[-0.3%, 0.0%]	85

Bootstrap: 749.811s -> 750.544s (0.10%)
Artifact size: 371.77 MiB -> 371.78 MiB (0.00%)

bjorn3 · 2025-06-05T17:39:46Z

This slightly improved the binary size. Likely because the string merging linker optimization now kicks in.

Add Location::file_with_nul This is useful for C/C++ APIs which expect the const char* returned from __FILE__ or std::source_location::file_name. ACP: rust-lang/libs-team#466 Tracking issue: rust-lang#141727

rustbot assigned cuviper Jan 3, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. labels Jan 3, 2025

RalfJung reviewed Jan 3, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/place.rs Outdated Show resolved Hide resolved

compiler/rustc_const_eval/src/util/caller_location.rs Show resolved Hide resolved

library/core/src/panic/location.rs Outdated Show resolved Hide resolved

cramertj force-pushed the file-cstr branch from f1b6278 to d070bb7 Compare January 3, 2025 17:20

cramertj changed the title ~~Add Location::file_cstr~~ Add Location::file_with_nul Jan 3, 2025

cramertj force-pushed the file-cstr branch 2 times, most recently from d28ac01 to 73810aa Compare January 3, 2025 17:24

cramertj mentioned this pull request Jan 3, 2025

ACP: Add nul-terminated version of core::panic::Location::file rust-lang/libs-team#466

Closed

ojeda mentioned this pull request Jan 3, 2025

core wanted features & bugfixes Rust-for-Linux/linux#514

Open

19 tasks

oli-obk reviewed Jan 7, 2025

View reviewed changes

compiler/rustc_const_eval/src/util/caller_location.rs Show resolved Hide resolved

This was referenced Jan 7, 2025

Add nul-terminated filename for #[track_caller] #131828

Closed

Add unstable option to nul-terminate location strings #135240

Closed

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 8, 2025

m-ou-se unassigned BurntSushi Jun 4, 2025

cramertj force-pushed the file-cstr branch from 73810aa to 88d782f Compare June 4, 2025 16:22

This comment has been minimized.

Sign in to view

Add Location::file_with_nul

b541f93

This is useful for C/C++ APIs which expect the const char* returned from __FILE__ or std::source_location::file_name.

cramertj force-pushed the file-cstr branch from 88d782f to b541f93 Compare June 4, 2025 16:23

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 4, 2025

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 5, 2025

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 5, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 5, 2025

bors merged commit c360e21 into rust-lang:master Jun 5, 2025
11 checks passed

rustbot added this to the 1.89.0 milestone Jun 5, 2025

cramertj deleted the file-cstr branch June 5, 2025 17:08

This was referenced Jun 10, 2025

panic::Location Debug implementation now prints a byte array #142279

Closed

Miscellaneous RefCell cleanups #142216

Merged

Uh oh!

Conversation

cramertj commented Jan 3, 2025 • edited by Darksonn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 3, 2025

Uh oh!

rustbot commented Jan 3, 2025

Uh oh!

Noratrieb commented Jan 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cramertj commented Jan 3, 2025

Uh oh!

bjorn3 commented Jan 5, 2025

Uh oh!

cramertj commented Jan 6, 2025

Uh oh!

Darksonn commented Jan 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cramertj commented Jan 7, 2025

Uh oh!

Darksonn commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Noratrieb commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

bors commented Jan 8, 2025

Uh oh!

bors commented Jan 8, 2025

Uh oh!

This comment has been minimized.

m-ou-se commented Jun 4, 2025

Uh oh!

bors commented Jun 4, 2025

Uh oh!

bors commented Jun 5, 2025

Uh oh!

rust-log-analyzer commented Jun 5, 2025

Uh oh!

bors commented Jun 5, 2025

Uh oh!

m-ou-se commented Jun 5, 2025

Uh oh!

bors commented Jun 5, 2025

Uh oh!

bors commented Jun 5, 2025

Uh oh!

Uh oh!

github-actions bot commented Jun 5, 2025

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Jun 5, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

bjorn3 commented Jun 5, 2025

Uh oh!

Reviewers

cramertj commented Jan 3, 2025 •

edited by Darksonn

Loading

Darksonn commented Jan 8, 2025 •

edited

Loading

Noratrieb commented Jan 8, 2025 •

edited

Loading