rustc_span: Generate keyword classification functions automatically

[petrochenkov]

The generated functions are compatible with symbols not forming continuous ranges.
This is a nicer alternative to 4896614 from #74554.
r? @nnethercote

[petrochenkov]

@bors try @rust-timer queue

The classification functions are array searches rather than simpler range checks, so they are supposed to be slower.
However, parsing is fast in general, so this shouldn't have a noticeable effect?

For small lengths (like those of the keyword lists) linear search is preferable to binary search.
For optimal speed the arrays should be sorted (to please branch predictor), but this would require copypasting some logic from rustc_span to rustc_macros after #74554 is merged.

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit 260234aa5f640d8f080e84a58d9292900b3f7348 with merge 2e0259109d9ebdf5660e31aa7bce983d3108f27c...

[Mark-Simulacrum]

It does seem a bit unfortunate to lose the O(1) range-checking here, but I guess this is probably not too hot.

Maybe LLVM takes care of things for us, too, though I somewhat doubt it.

Edit: Oh, I see your comment now. We can wait on perf.

I was expecting that we'd lower to #class_stream[0] <= self && self <= #class_stream.last, though obviously that syntax isn't actually a thing.

[petrochenkov]

This PR is supposed to land only together with #74554, which also regresses performance due to inability to do range checks, but improves it more due to inlining.

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 2e0259109d9ebdf5660e31aa7bce983d3108f27c (2e0259109d9ebdf5660e31aa7bce983d3108f27c)

[rust-timer]

Queued 2e0259109d9ebdf5660e31aa7bce983d3108f27c with parent 1facd4a, future comparison URL.

[rust-timer]

Finished benchmarking try commit (2e0259109d9ebdf5660e31aa7bce983d3108f27c): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

[Mark-Simulacrum]

Seems like a definite slow down, but mostly only affecting doc builds (1-2% on smaller crates). <1% regression in instruction counts on non-doc builds.

[nnethercote]

Yeah, maybe we shouldn't land this until we are certain about #74554?

[petrochenkov]

I wonder if there's any simpler way to skip a token with syn parser.

[petrochenkov]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit 1003529c05c7e35faea2e246758d77b7151853ce with merge a69ffd7a844a5bb0dea3220cceeb958ab7a2d3ad...

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: a69ffd7a844a5bb0dea3220cceeb958ab7a2d3ad (a69ffd7a844a5bb0dea3220cceeb958ab7a2d3ad)

[rust-timer]

Queued a69ffd7a844a5bb0dea3220cceeb958ab7a2d3ad with parent f50f1c8, future comparison URL.

[rust-timer]

Finished benchmarking try commit (a69ffd7a844a5bb0dea3220cceeb958ab7a2d3ad): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

[petrochenkov]

Ok, match looks better than contains (but is still a regression).

[petrochenkov]

This confuses code highlighting in my editor (vscode + rust-analyzer), but fn is required for searchability.
Perhaps the list needs to go into braces, e.g. like this:

fn is_special {
    Invalid:            "",
    PathRoot:           "{{root}}",
    DollarCrate:        "$crate",
    Underscore:         "_",
}

[petrochenkov]

error[E0107]: wrong number of type arguments: expected 3, found 2
  --> src/librustc_macros/src/symbols.rs:50:17
   |
50 | struct Keywords(IndexMap<Option<Ident>, Vec<Keyword>>);
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 3 type arguments

Excuse me?
Somehow rustc_macros gets built without has_std feature being enabled for the indexmap crate.

[Mark-Simulacrum]

Hm so indexmap is using autocfg to probe for std's presence, I think via this bit of code. My guess is that's not using the right rustc or not passing it the right parameters, but I'm not sure without seeing the actual build log from autocfg we'd be able to say much.

[petrochenkov]

I couldn't reproduce the indexmap error locally, but fixed it by providing the hasher argument explicitly.

[shepmaster]

My guess is that's not using the right rustc or not passing it the right parameters

See also rust-lang/cargo#8646

[petrochenkov]

So, this PR should probably just be cherry-picked into #74554 since it's not valuable by itself.

[bors]

☔ The latest upstream changes (presumably #74862) made this pull request unmergeable. Please resolve the merge conflicts.

[petrochenkov]

Closing since #74554 has been closed.