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.

Sign up for GitHub

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

Jump to bottom

Always include a position span in rustc_parse_format::Argument #99987

Merged
merged 1 commit into from
Aug 2, 2022
Merged

Always include a position span in rustc_parse_format::Argument #99987

merged 1 commit into from
Aug 2, 2022

Conversation

Alexendoo
Copy link
Member

Moves the spans from the Position enum to always be included in the Argument struct. Doesn't make any changes to use it in rustc, but it will be useful for some upcoming Clippy lints

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 31, 2022
@rustbot
Copy link
Collaborator

rustbot commented Jul 31, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-highfive
Copy link
Collaborator

r? @fee1-dead

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2022
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
  IMAGE: x86_64-gnu-tools
##[endgroup]
From https://github.com/rust-lang/rust
 * branch              master     -> FETCH_HEAD
Searching for toolstate changes between 76822a28780a9a93be04409e52c5df21663aab97 and 727c6f680719862474526a9bea174e7aafa260d4
Clippy or rustfmt subtrees were updated
##[group]Run src/ci/scripts/verify-channel.sh
src/ci/scripts/verify-channel.sh
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
---
.......... (60/67)
.......    (67/67)


/checkout/src/test/rustdoc-gui/default-settings.goml An exception occured: connect ECONNREFUSED 127.0.0.1:41411
== STACKTRACE ==
Error
    at innerRunTestCode (/node-v14.4.0-linux-x64/lib/node_modules/browser-ui-test/src/index.js:533:16)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async main (/checkout/src/tools/rustdoc-gui/tester.js:242:13)

@Alexendoo
Copy link
Member Author

Looks spurious, I hit re-run

@compiler-errors
Copy link
Member

compiler-errors commented Jul 31, 2022 •
edited
Loading

cc @PrestonFrom, you may or may not care about this but it's code you've recently touched

@PrestonFrom
Copy link
Contributor

Looks like a good change. Curious is adding the span to position instead of argument makes sense?

@Alexendoo
Copy link
Member Author

I moved it up into Argument as having a span on every Position variant felt a bit awkward, it's also consistent with FormatSpec's thing + thing_span

@PrestonFrom
Copy link
Contributor

That makes sense to me!

@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 2, 2022

📌 Commit 2a0b51d has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2022
fee1-dead
fee1-dead approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks

@PrestonFrom PrestonFrom mentioned this pull request Aug 2, 2022
Merged
@Alexendoo Alexendoo mentioned this pull request Aug 2, 2022
Merged
@Dylan-DPC Dylan-DPC mentioned this pull request Aug 2, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 2, 2022
@Dylan-DPC
Rollup merge of rust-lang#99987 - Alexendoo:parse-format-position-spa…
98e9381 
…n, r=fee1-dead

Always include a position span in `rustc_parse_format::Argument`

Moves the spans from the `Position` enum to always be included in the `Argument` struct. Doesn't make any changes to use it in rustc, but it will be useful for some upcoming Clippy lints
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 2, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 2, 2022
@bors
Auto merge of rust-lang#100063 - matthiaskrgr:rollup-lznouys, r=matth…
4493a0f 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#99987 (Always include a position span in `rustc_parse_format::Argument`)
 - rust-lang#100005 (Remove Clean trait for ast::Attribute and improve Attributes::from_ast)
 - rust-lang#100025 (Remove redundant `TransferWrapper` struct)
 - rust-lang#100045 (Properly reject the `may_unwind` option in `global_asm!`)
 - rust-lang#100052 (RISC-V ASM test: relax label name constraint.)
 - rust-lang#100053 (move [`assertions_on_result_states`] to restriction)
 - rust-lang#100057 (Remove more Clean trait implementations)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a0991b8 into rust-lang:master Aug 2, 2022
@rustbot rustbot added this to the 1.64.0 milestone Aug 2, 2022
@Alexendoo Alexendoo deleted the parse-format-position-span branch August 2, 2022 20:27
flip1995 pushed a commit to flip1995/rust that referenced this pull request Aug 11, 2022
@matthiaskrgr
Rollup merge of rust-lang#99987 - Alexendoo:parse-format-position-spa…
4546f5d 
…n, r=fee1-dead

Always include a position span in `rustc_parse_format::Argument`

Moves the spans from the `Position` enum to always be included in the `Argument` struct. Doesn't make any changes to use it in rustc, but it will be useful for some upcoming Clippy lints
@Alexendoo Alexendoo mentioned this pull request Aug 21, 2022
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 23, 2022
@Dylan-DPC
Rollup merge of rust-lang#100851 - Alexendoo:rpf-width-prec-spans, r=…
6a4fe5c 
…fee1-dead

Fix rustc_parse_format precision & width spans

When a `precision`/`width` was `CountIsName - {:name$}` or `CountIs - {:10}` the `precision_span`/`width_span` was set to `None`

For `width` the name span in `CountIsName(_, name_span)` had its `.start` off by one

r? `@fee1-dead` / cc `@PrestonFrom` since this is similar to rust-lang#99987
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 23, 2022
@Dylan-DPC
Rollup merge of rust-lang#100851 - Alexendoo:rpf-width-prec-spans, r=…
110d8d9 
…fee1-dead

Fix rustc_parse_format precision & width spans

When a `precision`/`width` was `CountIsName - {:name$}` or `CountIs - {:10}` the `precision_span`/`width_span` was set to `None`

For `width` the name span in `CountIsName(_, name_span)` had its `.start` off by one

r? ``@fee1-dead`` / cc ``@PrestonFrom`` since this is similar to rust-lang#99987
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fee1-dead fee1-dead fee1-dead approved these changes

Assignees

@fee1-dead fee1-dead

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@Alexendoo @rustbot @rust-highfive @rust-log-analyzer @compiler-errors @PrestonFrom @fee1-dead @bors