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

Remove noisy suggestion #72947

Closed
wants to merge 1 commit into from
Closed

Remove noisy suggestion #72947

wants to merge 1 commit into from

Conversation

ayushmishra2005
Copy link
Contributor

Remove noisy suggestion #72642

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(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 Jun 3, 2020
davidtwco
davidtwco requested changes Jun 3, 2020
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

This is great, super close to what I think we'd want!

src/test/ui/resolve/use_suggestion_placement.stderr Show resolved Hide resolved
src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved
@ayushmishra2005
Copy link
Contributor Author

I am working on to fix the failing test-cases

@ayushmishra2005 ayushmishra2005 requested review from davidtwco and lcnr June 3, 2020 17:07
davidtwco
davidtwco approved these changes Jun 3, 2020
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

LGTM, might be worth squashing the commits but otherwise great! Thanks!

@ayushmishra2005 ayushmishra2005 force-pushed the remove_noisy_suggestion branch from c010e22 to 968594d Compare June 3, 2020 23:43
@ayushmishra2005 ayushmishra2005 requested a review from davidtwco June 3, 2020 23:45
@ayushmishra2005
Copy link
Contributor Author

LGTM, might be worth squashing the commits but otherwise great! Thanks!

@davidtwco Done sir :) Please re-approve.

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 4, 2020

📌 Commit 968594d has been approved by davidtwco

@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 Jun 4, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 4, 2020
@Dylan-DPC
Rollup merge of rust-lang#72947 - ayushmishra2005:remove_noisy_sugges…
84e1c78 
…tion, r=davidtwco

Remove noisy suggestion

Remove noisy suggestion rust-lang#72642
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Jun 4, 2020
Closed
@Dylan-DPC-zz
Copy link

failed in rollup

@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 Jun 4, 2020
@ayushmishra2005
Copy link
Contributor Author

@Dylan-DPC Please let me know if anything is pending from my side.

@tesuji
Copy link
Contributor

tesuji commented Jun 5, 2020

Some tests may need to be blessed: https://dev.azure.com/rust-lang/rust/_build/results?buildId=31078&view=logs&j=698b0e4a-0480-533d-81fa-f38599aa328e&t=1bb3df5b-0a90-5853-4e9c-949e3efa722f&l=1576

@lcnr
Copy link
Contributor

lcnr commented Jun 5, 2020

When you have to update to master, please use git rebase in the future. See https://rustc-dev-guide.rust-lang.org/conventions.html#how-to-structure-your-pr for more details.

I think the easiest way to fix this is to once again squash the commits.

Remove noisy suggestion of hash_map rust-lang#72642
d8d57f5 
Update src/librustc_resolve/diagnostics.rs

Co-authored-by: David Wood <Q0KPU0H1YOEPHRY1R2SN5B5RL@david.davidtw.co>

Minor refactoring rust-lang#72642

Fixed failing test-cases

remove trivial `mk_predicate`s

Make `SourceMap` available for early debug-printing of `Span`s

Normally, we debug-print `Spans` using the `SourceMap` retrieved from
the global `TyCtxt`. However, we fall back to printing out the `Span`'s
raw fields (instead of a file and line number) when we try to print a
`Span` before a `TyCtxt` is available. This makes debugging early phases
of the compile, such as parsing, much more difficult.

This commit stores a `SourceMap` in `rustc_span::GlOBALS` as a fallback.
When a `TyCtxt` is not available, we try to retrieve one from `GLOBALS`
- only if this is not available do we fall back to the raw field output.

I'm not sure how to write a test for this - however, this can be
verified locally by setting `RUSTC_LOG="rustc_parse=debug"`, and
verifying that the output contains filenames and line numbers.

Add test for rust-lang#72554.

rustc_target: Remove `pre_link_args_crt`

Improve E0433, so that it suggests missing imports

Fix a typo in `late.rs`

Co-authored-by: Esteban Kuber <estebank@users.noreply.github.com>

fix `AdtDef` docs

Add Camelid

Email from @camelid:

> HI there,
>
> I’m a new contributor and I just looked at Rust Thanks and noticed that my contributions are listed under two different capitalizations of my name: “Camelid" and “camelid". Could you make them both “Camelid"?
>
> Thanks!
>
> Camelid

save_analysis: work on HIR tree instead of AST

Update `rls` submodule

remove outdated fixme

Hexagon libstd: fix typo for c_ulonglong

Add more assert to Vec with_capacity docs

Show assertion on len too to show them how adding new items will affect both the
length and capacity, before and after.

Add Kyle Strand to mailmap

Fix missing word in RELEASES.md

Update cargo

Enable lld for Cargo tests on Windows.

Fixed failing test-cases rust-lang#72642
@ayushmishra2005 ayushmishra2005 force-pushed the remove_noisy_suggestion branch from 5a6f754 to d8d57f5 Compare June 5, 2020 09:18
@ayushmishra2005
Copy link
Contributor Author

I am closing this pull request. I will raise the new pull request.

@ayushmishra2005 ayushmishra2005 deleted the remove_noisy_suggestion branch June 5, 2020 09:19
@ayushmishra2005 ayushmishra2005 mentioned this pull request Jun 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidtwco davidtwco davidtwco approved these changes

@lcnr lcnr Awaiting requested review from lcnr

Assignees

@davidtwco davidtwco

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ayushmishra2005 @rust-highfive @davidtwco @bors @Dylan-DPC-zz @tesuji @lcnr