Fix Ord, Eq and Hash implementation of panic::Location #144510
Conversation
|
r? @ibraheemdev rustbot has assigned @ibraheemdev. Use |
rustbot
added
S-waiting-on-review
| self.col.hash(state); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you add some test coverage for these? That would help prevent future accidental regressions.
There was a problem hiding this comment.
I'm actually not 100% sure if I can? In what little tests I've written for the standard library it's been single-file-per-test.
There was a problem hiding this comment.
You should be able to easily add some tests into library/coretests somewhere, I believe. Although you will need to have some sort of perma-unstable method to construct these that's marked #[doc(hidden)], since these are private fields.
There was a problem hiding this comment.
Location::internal_constructor does just that :) and there is an existing test file at library/coretests/tests/panic/___location.rs
There was a problem hiding this comment.
It appears that the function you're referencing no longer exists and was removed when the file was converted to a non-null pointer instead of a real string. Specifically here: b541f93
There was a problem hiding this comment.
Interesting; I guess it could be brought back as a function taking a &CStr.
There was a problem hiding this comment.
I've added tests now using some extra helper modules in separate files, so the tests can remain integration rather than needing specific test-only hidden constructors.
|
#142708 landed in 1.90 and I think that's what introduced the bug, so presuming this doesn't land by ~Friday we should backport it to future beta. Not nominating yet since it'll cause confusion until we actually branch. |
This comment has been minimized.
This comment has been minimized.
|
Thanks for the fix. @bors r+ rollup |
bors
added
S-waiting-on-bors
|
squash? |
bors
added
S-waiting-on-author
Faster equality compare Add tests Add missing files for tests
orlp
force-pushed
the
fix-___location-ord
branch
from
0e6615d to
05da623
Compare
|
@ibraheemdev Done. |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 8 pull requests Successful merges: - #144034 (tests: Test line number in debuginfo for diverging function calls) - #144510 (Fix Ord, Eq and Hash implementation of panic::Location) - #144583 (Enable T-compiler backport nomination) - #144586 (Update wasi-sdk to 27.0 in CI) - #144605 (Resolve: cachify `ExternPreludeEntry.binding` through a `Cell`) - #144632 (Update some tests for LLVM 21) - #144639 (Update rustc-perf submodule) - #144640 (Add support for the m68k architecture in 'object_architecture') r? `@ghost` `@rustbot` modify labels: rollup
Fix Ord, Eq and Hash implementation of panic::Location Fixes rust-lang#144486. Now properly compares/hashes the filename rather than the pointer to the string.
Rollup of 8 pull requests Successful merges: - rust-lang#144034 (tests: Test line number in debuginfo for diverging function calls) - rust-lang#144510 (Fix Ord, Eq and Hash implementation of panic::Location) - rust-lang#144583 (Enable T-compiler backport nomination) - rust-lang#144586 (Update wasi-sdk to 27.0 in CI) - rust-lang#144605 (Resolve: cachify `ExternPreludeEntry.binding` through a `Cell`) - rust-lang#144632 (Update some tests for LLVM 21) - rust-lang#144639 (Update rustc-perf submodule) - rust-lang#144640 (Add support for the m68k architecture in 'object_architecture') r? `@ghost` `@rustbot` modify labels: rollup
Fixes #144486.
Now properly compares/hashes the filename rather than the pointer to the string.