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

Unhide unknown spans #103970

Merged
merged 3 commits into from
Nov 12, 2022
Merged

Unhide unknown spans #103970

merged 3 commits into from
Nov 12, 2022

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Nov 4, 2022

@rustbot
Copy link
Collaborator

rustbot commented Nov 4, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 4, 2022
@rustbot
Copy link
Collaborator

rustbot commented Nov 4, 2022

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@davidtwco
Copy link
Member

r? @davidtwco

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 7, 2022

📌 Commit 9268133 has been approved by davidtwco

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 Nov 7, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 7, 2022
@GuillaumeGomez
Rollup merge of rust-lang#103970 - oli-obk:unhide_unknown_spans, r=da…
ac8d5c9 
…vidtwco

Unhide unknown spans

r? `@estebank`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Nov 7, 2022
Closed
@GuillaumeGomez
Copy link
Member

Failed in #104123.

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 7, 2022
@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 11, 2022

I re-added the label, but avoid emitting it in case of dummy spans.

@estebank
Copy link
Contributor

I re-added the label, but avoid emitting it in case of dummy spans.

Makes sense! Now I'm intrigued as to why some of those where DUMMY_SP to begin with, nothing I saw in the previous tests gave me the impression they'd be, but I'm ok with proactive checking. The whole diagnostics machinery should treat all spans as Option<Span> and give enough tools to decide what to show, instead of what we're doing now :-/

@bors r+

@bors
Copy link
Contributor

bors commented Nov 11, 2022

📌 Commit df2adc4 has been approved by estebank

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 11, 2022
@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 11, 2022

Yea... the idents should have spans, but I didn't quickly find anything so I opted for this instead.

Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 11, 2022
@Manishearth
Rollup merge of rust-lang#103970 - oli-obk:unhide_unknown_spans, r=es…
20d7207 
…tebank

Unhide unknown spans

r? `@estebank`
@Manishearth Manishearth mentioned this pull request Nov 11, 2022
Closed
@Manishearth Manishearth mentioned this pull request Nov 11, 2022
Closed
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 11, 2022
@Manishearth
Rollup merge of rust-lang#103970 - oli-obk:unhide_unknown_spans, r=es…
06244ed 
…tebank

Unhide unknown spans

r? ``@estebank``
@Dylan-DPC Dylan-DPC mentioned this pull request Nov 12, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 12, 2022
@bors
Auto merge of rust-lang#104310 - Dylan-DPC:rollup-wgt1z4a, r=Dylan-DPC
5f4e73c 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#102049 (Add the `#[derive_const]` attribute)
 - rust-lang#103970 (Unhide unknown spans)
 - rust-lang#104206 (Remove `save_and_restore_in_snapshot_flag`, use `ObligationCtxt` more)
 - rust-lang#104214 (Emit error in `collecting_trait_impl_trait_tys` on mismatched signatures)
 - rust-lang#104267 (rustdoc: use checkbox instead of switch for settings toggles)
 - rust-lang#104302 (Update cargo)
 - rust-lang#104303 (UI tests can be assigned to T-compiler)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fcbe990 into rust-lang:master Nov 12, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 12, 2022
@oli-obk oli-obk deleted the unhide_unknown_spans branch November 13, 2022 13:32
@oli-obk oli-obk mentioned this pull request Nov 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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.67.0
Development

Successfully merging this pull request may close these issues.
6 participants
@oli-obk @rustbot @davidtwco @bors @GuillaumeGomez @estebank