Add nul-terminated filename for #[track_caller]#131828
Add nul-terminated filename for #[track_caller]#131828Darksonn wants to merge 4 commits intorust-lang:masterfrom
Conversation
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
| )] | ||
| impl<'a> Location<'a> { | ||
| #[doc(hidden)] | ||
| pub const fn internal_constructor(file: &'a str, line: u32, col: u32) -> Self { |
There was a problem hiding this comment.
Note that this internal constructor does not seem to be used anymore.
This comment has been minimized.
This comment has been minimized.
|
see #117431, this is a small binary size improvement if we then make use of the nul terminator (probably only worth it after the bootstrap bump) |
|
Yeah, getting rid of the length could reduce the size of |
This comment has been minimized.
This comment has been minimized.
|
My PR implements the bootstrapping part correctly (including the place where the Location constant is created), but the whole byte allocation stuff is wrong. I recommend you copy the library part and Location creation from my PR and keep the string allocator from yours. |
|
Based on the above failure, it seems like my allocation stuff is also wrong? Does it get allocated in multiple places? |
|
hm, not sure. not understanding the problems with the allocation stuff is why I quit my PR, so I can't help you there sadly :3 |
|
@Noratrieb Nevermind, looks like I was just missing a nul-terminator in the |
|
it seems correct to me. i guess it makes more sense to do the size reduction separately in the future. |
|
This seems like a good idea! |
jhpratt
left a comment
There was a problem hiding this comment.
r=me if/when the ACP gets accepted.
|
☔ The latest upstream changes (presumably #134096) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Closing in favor of #135054 as this PR is outdated. |
ACP: Add nul-terminated version of core::panic::Location::file
When using
#[track_caller]you can get the filename of the caller usingLocation::caller().file(). We would like to utilize this in the Linux kernel to implement a Rust equivalent of the following utility:It's essentially an assertion that crashes the kernel if a function is used in the wrong context. The filename and line number is used in the error message when it fails. Unfortunately, the
__might_sleepfunction requires the filename to be a nul-terminated string. Thus, extendLocationwith a function that returns the filename as a&CStr.Note that unlike with things like the
file!()macro, it's impossible for us to do this ourselves statically. Copying the filename into another string to nul-terminate it is not a great solution because we need to create the string even if the assertion doesn't fail, as the assertion happens on the C side.For more context, please see zulip and the Linux kernel mailing list. This is one of RfL's wanted features in core.
cc @ojeda @Urgau