rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.57k stars 12.74k forks source link

Debug info for const generics is hashes rather than something nice #115786

Open RalfJung opened 1 year ago

RalfJung commented 1 year ago

In https://github.com/rust-lang/rust/pull/115748 I got a test failure in tests/debuginfo/function-names.rs: function_names::const_generic_fn_non_int<{CONST#ad91263f6d2dd96e}> changed to static fn function_names::const_generic_fn_non_int<{CONST#73a38766b652ae7d}>. That PR leads to some new AllocIds being generated which shifts some IDs around, so it seems like the AllocId are affecting the function name here. @oli-obk was confused by this since with valtrees, shouldn't our function names be stable now? Indeed AllocId are not very stable at all so it seems potentially problematic if they are relevant for function names. The underlying function call is const_generic_fn_non_int::<{ () }> so this is not a very complicated valtree. ;)

Cc @rust-lang/project-const-generics

RalfJung commented 1 year ago

Actually I am seeing the name change in https://github.com/rust-lang/rust/pull/115803 as well, so it's not some alloc ID change, it's... maybe a slightly different representation of the same const value?

RalfJung commented 1 year ago

Mystery solved! It's this hash right here:

https://github.com/rust-lang/rust/blob/e16a6d003b0f99823d8bbce349456389f7012ba2/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs#L664-L670

Now, whether that's expected behavior or not, I cannot say.

oli-obk commented 1 year ago

Ah yea... I guess I should check how we render strings. It's probably an issue if we have symbol names that themselves are multiple megabytes large

RalfJung commented 1 year ago

Does https://github.com/rust-lang/rfcs/pull/3161 fix this? Not sure if v0 mangling is involved and what happens for old-school mangling (or if old-school mangling is even still a thing).

EDIT: The PR doesn't change the relevant test, so I guess the answer is "no". Are there any plans to completely switch to v0 name mangling, or do we need a similar fix for old name mangling?

michaelwoerister commented 4 months ago

FYI: these names are part of debuginfo and not related to symbol mangling. Any change to the debuginfo version of these names should be done with care to make sure we don't break GDB, LLDB, NatVis, etc.

BoxyUwU commented 4 months ago

cc #126060