Fix generics where bounds order

[GuillaumeGomez]

Fixes #88809.

Like said on the above issue, the issue is that we were expecting Symbol comparisons to be string-based but they are integer-based (because Symbol is an integer), messing up the bounds order. To fix it, I simply stored into a FxIndexMap instead.

r? @jyn514

[GuillaumeGomez]

I implemented @Mark-Simulacrum's suggestion.

[camelid]

@GuillaumeGomez This bug seems to be cross-crate-export related, so you'll need to re-export a type implementing a trait to reproduce this. I came up with a repro that should work as a src/test/rustdoc/ test (with an auxiliary file) if you add the appropriate @has checks:

// In lib1.rs

pub struct Struct<A, B, C>(A, B, C);

// NOTE: would it make sense to define `struct B`, `struct A`, `struct C`, in that order,
// to make sure the symbol indices are in a random order?

pub trait Trait<Other> {}

impl<A, B, C> Trait<Struct<A, B, C>> for Struct<A, B, C>
where
    A: Trait<A>,
    B: Trait<B>,
    C: Trait<C>,
{
}

// In lib2.rs

// aux-build:lib1.rs

pub use lib1::*;

// Add `@has` checks here, either for `Struct`'s page, or for `Trait`'s page

[camelid]

Like said on the above issue, the issue is that we were expecting Symbol comparisons to be string-based but they are integer-based (because Symbol is an integer), messing up the bounds order. To fix it, I simply stored the original order and then iterate over this one instead.

About the implementation: one idea I had was to simply switch from a BTreeMap to a Vec instead of having both.

Also, could you update the PR description to reflect that this now uses FxIndexMap?

[GuillaumeGomez]

@camelid: Just realized that we can't check that in test/rustdoc because the implementors content is generated through HTML.

[camelid]

@camelid: Just realized that we can't check that in test/rustdoc because the implementors content is generated through HTML.

Ah, that's unfortunate. I think it might not have been possible anyway because I don't know if there's a way to check ordering of elements with @has.

[camelid]

@camelid: Just realized that we can't check that in test/rustdoc because the implementors content is generated through HTML.

Actually, I don't think it is. You can check the implementors section of lib2::Trait or the implementations of lib2::Struct – they both work with JS disabled AFAICT. Both the trait and the struct are re-exported, so if you use the re-exported versions, it should work.

However, there's still the challenge of checking the order of the HTML elements in the where clause. Is there a way to do that?

[GuillaumeGomez]

We could add this check with a better XPath support:

// @has	- '//*[@id="implementors-list"]//span[@class="where fmt-newline"]/text()[1]' 'B:'

Unfortunately, it's not supported:

Traceback (most recent call last):
  File "/usr/lib/python3.9/xml/etree/ElementPath.py", line 354, in iterfind
    selector = _cache[cache_key]
KeyError: ('.//*[@id="implementors-list"]//span[@class="where fmt-newline"]/text()[1]',)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/imperio/rust/rust/src/etc/htmldocck.py", line 490, in <module>
    check(sys.argv[1], get_commands(sys.argv[2]))
  File "/home/imperio/rust/rust/src/etc/htmldocck.py", line 482, in check
    check_command(c, cache)
  File "/home/imperio/rust/rust/src/etc/htmldocck.py", line 439, in check_command
    ret = check_tree_text(cache.get_tree(c.args[0]), pat, c.args[2], regexp)
  File "/home/imperio/rust/rust/src/etc/htmldocck.py", line 370, in check_tree_text
    for e in tree.findall(path):
  File "/usr/lib/python3.9/xml/etree/ElementPath.py", line 395, in findall
    return list(iterfind(elem, path, namespaces))
  File "/usr/lib/python3.9/xml/etree/ElementPath.py", line 368, in iterfind
    selector.append(ops[token[0]](next, token))
KeyError: '()'

[GuillaumeGomez]

The python module we use has been deprecated a while ago. We should switch to another parser like BeautifulSoup which supports XPath entirely, that would make such checks possible.

What do you think @jyn514 ? If we agree on that, let's approve this one, open an issue to add a test whenever it's possible and I'll send a PR to switch the HTML parser we use.

[camelid]

@bors r+ rollup

[bors]

📌 Commit b226d17 has been approved by camelid