-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
collect_tokens_trailing_token
cleanups
#127902
collect_tokens_trailing_token
cleanups
#127902
Conversation
This comment has been minimized.
This comment has been minimized.
01ce3e9
to
b21933e
Compare
This comment has been minimized.
This comment has been minimized.
r=me after updating the doc example to not fail. |
☔ The latest upstream changes (presumably #127906) made this pull request unmergeable. Please resolve the merge conflicts. |
b21933e
to
4b20da7
Compare
I fixed the doc failure and rebased. @bors r=petrochenkov |
@bors rollup |
…g_token-cleanups, r=petrochenkov `collect_tokens_trailing_token` cleanups More cleanups I made while understanding the code for processing `cfg_attr`, to fix test failures in rust-lang#124141. r? ``@petrochenkov``
@nnethercote I guess you have to rebase again and update the No clue why that didn't show up in PR CI, #127936 is the only thing that merged since you rebased. @bors r- |
The `Option`s within the `ReplaceRange`s within the hashmap are always `None`. This PR omits them and inserts them when they are extracted from the hashmap.
It's no shorter than `ret.attrs()`, and `ret.attrs()` is used multiple times earlier in the function.
This puts it just before the `replace_ranges` initialization, which makes sense because the two variables are closely related.
Currently in `collect_tokens_trailing_token`, `start_pos` and `end_pos` are 1-indexed by `replace_ranges` is 0-indexed, which is really confusing. Making them both 0-indexed makes debugging much easier.
Adding details, clarifying lots of little things, etc. In particular, the commit adds details of an example. I find this very helpful, because it's taken me a long time to understand how this code works.
4b20da7
to
1dd566a
Compare
I added a reason to the @bors r=petrochenkov |
Rollup of 7 pull requests Successful merges: - rust-lang#121533 (Handle .init_array link_section specially on wasm) - rust-lang#127825 (Migrate `macos-fat-archive`, `manual-link` and `archive-duplicate-names` `run-make` tests to rmake) - rust-lang#127891 (Tweak suggestions when using incorrect type of enum literal) - rust-lang#127902 (`collect_tokens_trailing_token` cleanups) - rust-lang#127928 (Migrate `lto-smoke-c` and `link-path-order` `run-make` tests to rmake) - rust-lang#127935 (Change `binary_asm_labels` to only fire on x86 and x86_64) - rust-lang#127953 ([compiletest] Search *.a when getting dynamic libraries on AIX) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#127902 - nnethercote:collect_tokens_trailing_token-cleanups, r=petrochenkov `collect_tokens_trailing_token` cleanups More cleanups I made while understanding the code for processing `cfg_attr`, to fix test failures in rust-lang#124141. r? `@petrochenkov`
More cleanups I made while understanding the code for processing
cfg_attr
, to fix test failures in #124141.r? @petrochenkov