Skip to content

panic::Location ordering is defined by pointer ordering, not string ordering #144486

@clarfonthey

Description

@clarfonthey

Based upon the definition here:

pub struct Location<'a> {
    // A raw pointer is used rather than a reference because the pointer is valid for one more byte
    // than the length stored in this pointer; the additional byte is the NUL-terminator used by
    // `Location::file_with_nul`.
    filename: NonNull<str>,
    line: u32,
    col: u32,
    _filename: PhantomData<&'a str>,
}

This has a derived Ord (and subtraits) impl, which means it uses the Ord (and subtraits) impl for NonNull<str> instead of str, which uses pointer-based ordering. This feels incorrect, and I noticed it when trying to make its ordering impls const. The utility of making the ordering const is questionable, but regardless, this feels like an incorrect impl.

I'm filing this as an issue since this feels like it deserves a proper libs decision on whether this ordering should be preserved, or updated to be correct. Plus, maybe a crater run to ensure nobody cares about the fact that it uses pointer-ordering instead of string-ordering.

For the PartialEq impl, this doesn't matter because the location string is the same for each file, but for Ord, it definitely does matter.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-panicArea: Panicking machineryC-bugCategory: This is a bug.I-libs-api-nominatedNominated for discussion during a libs-api team meeting.T-libsRelevant to the library team, which will review and decide on the PR/issue.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions