-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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