Skip to content
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

Closed
uweigand opened this issue Dec 6, 2022 · 0 comments · Fixed by #106008
Closed

Lint suggestions depend on host endianness #105379

uweigand opened this issue Dec 6, 2022 · 0 comments · Fixed by #106008

Comments

@uweigand
Copy link
Contributor

uweigand commented Dec 6, 2022

When running ./x.py test on the big-endian s390x platform, I see the following error:

---- [ui] src/test/rustdoc-ui/unknown-renamed-lints.rs stdout ----
diff of stderr:

14        --> $DIR/unknown-renamed-lints.rs:7:9
15         |
16      LL | #![deny(rustdoc::x)]
-          |         ^^^^^^^^^^ help: did you mean: `rustdoc::all`
+          |         ^^^^^^^^^^ help: did you mean: `rustdoc`
18

Investigating this in more detail shows that this happens due to a combination of several factors:

  • The suggestion is generated by the no_lint_suggestion routine in compiler/rustc_lint/src/context.rs, which does
        let groups = self.lint_groups.keys().copied().map(Symbol::intern);
        let lints = self.lints.iter().map(|l| Symbol::intern(&l.name_lower()));
        let names: Vec<Symbol> = groups.chain(lints).collect();
        let suggestion = find_best_match_for_name(&names, Symbol::intern(&name_lower), None);
  • The find_best_match_for_name routine returns (in this case) the name out of names that has the lowest Levenshtein distance from the target name.
  • However, both rustdoc and rustdoc::all are at the same distance from rustdoc::x. In this case, the name that occurs earlier in the names list is returned - so the order of that vector matters.
  • self.lint_groups.keys() returns the elements of lint_groups in the order of the hash values of the FxHashMap. However, the associated Hasher in the rustc_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.
  • As a result, for this particular test case, the order of rustdoc and rustdoc::all in names 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:

  • Should the members of names be sorted according to some criteria to ensure deterministic diagnostics across platforms?
  • As an aside: rustdoc is considered "deprecated" while rustdoc::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.

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
@bors bors closed this as completed in 30fbfd5 Dec 22, 2022
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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant