-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Migrate rustc_resolve to use SessionDiagnostic, part # 1 #101162
Conversation
cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki |
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @fee1-dead (or someone else) soon. Please see the contribution instructions for more information. |
@rajputrajat, can you please remove all of these merge commits? We have a no merge commit policy. |
This comment has been minimized.
This comment has been minimized.
@rajputrajat -- I think you need to do something like |
Yes. I will update this as you suggested. Thanks! |
r? @davidtwco |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start, I've left some comments, you'll need to remove the merge commits as @compiler-errors has noted. :)
This comment was marked as resolved.
This comment was marked as resolved.
Some changes occurred in src/tools/cargo cc @ehuss The Miri submodule was changed cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Looks like something went wrong in the rebase, this contains a bunch of changes it shouldn't (submodules, lockfile). |
This comment has been minimized.
This comment has been minimized.
30f6b99
to
2f600ec
Compare
You should run |
Done. Please review. |
Wrong commits were picked while squashing. corrected! |
Some(errs::UnreachableLabelSubLabel { ident_span: ident.span }), | ||
Some(errs::UnreachableLabelSubSuggestion { | ||
span, | ||
ident_name: ident.name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..likewise here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one being used in 'suggestion', so seems like need to leave as is.
); | ||
err | ||
path_span: path.span, | ||
path_str: pprust::path_to_string(&path), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be another case where we can use IntoDiagnosticArg
and embed the meaningful type directly in the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that string is part of a suggestion, so need to leave as is.
Applicability::MaybeIncorrect, | ||
); | ||
err | ||
path_span: path.span, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note for myself: we should have a IntoDiagnosticSpan
trait and use that for all the various types that contain spans, like paths here or Ident
.
This comment has been minimized.
This comment has been minimized.
implement binding_shadows migrate till self-in-generic-param-default use braces in fluent message as suggested by @compiler-errors. to fix lock file issue reported by CI migrate 'unreachable label' error run formatter name the variables correctly in fluent file SessionDiagnostic -> Diagnostic test "pattern/pat-tuple-field-count-cross.rs" passed test "resolve/bad-env-capture2.rs" passed test "enum/enum-in-scope.rs" and other depended on "resolve_binding_shadows_something_unacceptable" should be passed now. fix crash errors while running test-suite. there might be more. then_some(..) suits better here. all tests passed convert TraitImpl and InvalidAsm. TraitImpl is buggy yet. will fix after receiving help from Zulip migrate "Ralative-2018" migrate "ancestor only" migrate "expected found" migrate "Indeterminate" migrate "module only" revert to the older implementation for now. since this is failing at the moment. follow the convension for fluent variable order the diag attribute as suggested in review comment fix merge error. migrate trait-impl-duplicate make the changes compatible with "Flatten diagnostic slug modules rust-lang#103345" fix merge remove commented code merge issues fix review comments fix tests
37a7e9d
to
269ce36
Compare
@bors r+ |
Migrate rustc_resolve to use SessionDiagnostic, part # 1 crate a somewhat on larger size, so plz allow some time to get it finished.
Migrate rustc_resolve to use SessionDiagnostic, part # 1 crate a somewhat on larger size, so plz allow some time to get it finished.
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#101162 (Migrate rustc_resolve to use SessionDiagnostic, part # 1) - rust-lang#103386 (Don't allow `CoerceUnsized` into `dyn*` (except for trait upcasting)) - rust-lang#103405 (Detect incorrect chaining of if and if let conditions and recover) - rust-lang#103594 (Fix non-associativity of `Instant` math on `aarch64-apple-darwin` targets) - rust-lang#104006 (Add variant_name function to `LangItem`) - rust-lang#104494 (Migrate GUI test to use functions) - rust-lang#104516 (rustdoc: clean up sidebar width CSS) - rust-lang#104550 (fix a typo) Failed merges: - rust-lang#104554 (Use `ErrorGuaranteed::unchecked_claim_error_was_emitted` less) r? `@ghost` `@rustbot` modify labels: rollup
☔ The latest upstream changes (presumably #104573) made this pull request unmergeable. Please resolve the merge conflicts. |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#101162 (Migrate rustc_resolve to use SessionDiagnostic, part # 1) - rust-lang#103386 (Don't allow `CoerceUnsized` into `dyn*` (except for trait upcasting)) - rust-lang#103405 (Detect incorrect chaining of if and if let conditions and recover) - rust-lang#103594 (Fix non-associativity of `Instant` math on `aarch64-apple-darwin` targets) - rust-lang#104006 (Add variant_name function to `LangItem`) - rust-lang#104494 (Migrate GUI test to use functions) - rust-lang#104516 (rustdoc: clean up sidebar width CSS) - rust-lang#104550 (fix a typo) Failed merges: - rust-lang#104554 (Use `ErrorGuaranteed::unchecked_claim_error_was_emitted` less) r? `@ghost` `@rustbot` modify labels: rollup
crate a somewhat on larger size, so plz allow some time to get it finished.