-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lint suggestions depend on host endianness #105379
Comments
JohnTitor
pushed a commit
to JohnTitor/rust
that referenced
this issue
Dec 21, 2022
…Nilstrieb Sort lint_groups in no_lint_suggestion The no_lint_suggestion routine passes a vector of lint group names to find_best_match_for_name. That routine depends on the sort order of its input vector, which matters in case multiple inputs are at the same Levenshtein distance to the target name. However, no_lint_suggestion currently just passes lint_groups.keys() as input vector - this is sorted in hash value order, which is not guaranteed to be stable, and in fact differs between big- and little-endian host platforms, causing test failures on s390x. To fix this, always sort the lint groups before using their names as input to find_best_match_for_name. In doing so, prefer non- deprecated lint group names over deprecated ones, and then use alphabetical order. Fixes rust-lang#105379
RalfJung
pushed a commit
to RalfJung/miri
that referenced
this issue
Dec 24, 2022
Sort lint_groups in no_lint_suggestion The no_lint_suggestion routine passes a vector of lint group names to find_best_match_for_name. That routine depends on the sort order of its input vector, which matters in case multiple inputs are at the same Levenshtein distance to the target name. However, no_lint_suggestion currently just passes lint_groups.keys() as input vector - this is sorted in hash value order, which is not guaranteed to be stable, and in fact differs between big- and little-endian host platforms, causing test failures on s390x. To fix this, always sort the lint groups before using their names as input to find_best_match_for_name. In doing so, prefer non- deprecated lint group names over deprecated ones, and then use alphabetical order. Fixes rust-lang/rust#105379
MaciejWas
pushed a commit
to MaciejWas/rust
that referenced
this issue
Dec 27, 2022
The no_lint_suggestion routine passes a vector of lint group names to find_best_match_for_name. That routine depends on the sort order of its input vector, which matters in case multiple inputs are at the same Levenshtein distance to the target name. However, no_lint_suggestion currently just passes lint_groups.keys() as input vector - this is sorted in hash value order, which is not guaranteed to be stable, and in fact differs between big- and little-endian host platforms, causing test failures on s390x. To fix this, always sort the lint groups before using their names as input to find_best_match_for_name. In addition, deprecated lint groups should never be suggested, so filter those out. Fixes rust-lang#105379
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When running
./x.py test
on the big-endians390x
platform, I see the following error:Investigating this in more detail shows that this happens due to a combination of several factors:
no_lint_suggestion
routine incompiler/rustc_lint/src/context.rs
, which doesfind_best_match_for_name
routine returns (in this case) the name out ofnames
that has the lowest Levenshtein distance from the target name.rustdoc
andrustdoc::all
are at the same distance fromrustdoc::x
. In this case, the name that occurs earlier in thenames
list is returned - so the order of that vector matters.self.lint_groups.keys()
returns the elements oflint_groups
in the order of the hash values of theFxHashMap
. However, the associatedHasher
in therustc_hash
crate does not guarantee stable values across different architecture; in particular, the same string will hash to different values on a big-endian platform vs. a little-endian platform.rustdoc
andrustdoc::all
innames
is different based on host endianness, and therefore the diagnostic emitted depends on host endianness as well.It seems to me this is not ideal. So I'm wondering:
names
be sorted according to some criteria to ensure deterministic diagnostics across platforms?rustdoc
is considered "deprecated" whilerustdoc::all
is not - should this routine ever suggest a deprecated name in the first place?I've implemented a fix that simply ignores deprecated members of
lint_groups
, which fixes the test case for me. Not sure if this is really the correct solution - I'd be happy to help implement and test other approaches as well.The text was updated successfully, but these errors were encountered: