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.

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

Add more span suggestions #89391

Closed
wants to merge 9 commits into from
Closed

Add more span suggestions #89391

wants to merge 9 commits into from

Conversation

Milo123459
Copy link
Contributor

Hey, first PR. This just adds some new span_suggestion's to some errors.

@rust-highfive
Copy link
Collaborator

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

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 30, 2021
@Milo123459
Copy link
Contributor Author

Milo123459 commented Sep 30, 2021

I was going to attempt to add something to the async closures are unstable error, but I'm not sure how to add another help section. It looks like a macro invocation (gate_all!) is being used

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@nagisa
Copy link
Member

nagisa commented Sep 30, 2021

r? @estebank

From my perspective a more descriptive PR title and description would be helfpul. Something like "Implement machine applicable suggestions for static closures and loop control flow constructs" for the title and describe why these are okay as machine-applicable hints in these situations.

@rust-highfive rust-highfive assigned estebank and unassigned nagisa Sep 30, 2021
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I left some inline comments asking for changes.

@estebank
Copy link
Contributor

estebank commented Oct 1, 2021

Also, you'll need to run ./x.py fmt before submitting, otherwise CI will complain and won't let us merge.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor

estebank commented Oct 4, 2021

You have a missing import after applying my suggested changes:

help: consider importing one of these items
    |
1   | use crate::path::BytePos;

@rust-log-analyzer

This comment has been minimized.

@estebank estebank 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 5, 2021
@Milo123459
Copy link
Contributor Author

Kinda confused as I added the use in and it still fails?

@bugaevc
Copy link

bugaevc commented Oct 14, 2021

In the static closure case, perhaps a better suggestion would be to use move? Assuming the person was trying to make the closure be 'static.

error[E0697]: cannot use `static` keyword on closures
  --> $DIR/E0697.rs:2:5
   |
LL |     static || {};
   |     ^^^^^^^^^ help: to make the closure capture its environment by
   |               value and have a `'static` lifetime, use the `move`
   |               keyword: `move || {}`
   = note: for more on capturing environment with closures, read https://doc.rust-lang.org/book/ch13-01-closures.html#capturing-the-environment-with-closures

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 14, 2021
@Milo123459
Copy link
Contributor Author

Hey - sorry I was gone, got caught up with something else. Let me fix this

@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 8, 2021
@jyn514
Copy link
Member

jyn514 commented Nov 9, 2021

@Milo123459 the error is in compiler/rustc_ast_lowering/src/expr.rs, but you only added the import to compiler/rustc_typeck/src/check/pat.rs. I recommend running x.py check locally, it will give much faster feedback and also the errors will be easier to read.

@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2021
@nagisa
Copy link
Member

nagisa commented Dec 31, 2021

Please undo the changes to Cargo.lock. Updates to the dependency versions should only occur as a separate and well informed decisions.

@Milo123459
Copy link
Contributor Author

Hm.. running git rebase master has done that..

@Milo123459
Copy link
Contributor Author

Please undo the changes to Cargo.lock. Updates to the dependency versions should only occur as a separate and well informed decisions.

Yes, I have tried that with git rebase: can't figure out what's going on here, I'm questioning why this even changed as well

@Milo123459
Copy link
Contributor Author

I think something locally has broken my branch 🤔

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustc_query_system v0.0.0 (/checkout/compiler/rustc_query_system)
    Checking rustc_parse v0.0.0 (/checkout/compiler/rustc_parse)
    Checking rustc_middle v0.0.0 (/checkout/compiler/rustc_middle)
    Checking rustc_ast_lowering v0.0.0 (/checkout/compiler/rustc_ast_lowering)
error[E0432]: unresolved import `rustc_span::hygiene::ForLoopLoc`
   |
   |
15 | use rustc_span::{hygiene::ForLoopLoc, BytePos, DUMMY_SP};
   |                  ^^^^^^^^^^^^^^^^^^^ no `ForLoopLoc` in `hygiene`
For more information about this error, try `rustc --explain E0432`.
error: could not compile `rustc_ast_lowering` due to previous error
warning: build failed, waiting for other jobs to finish...
error: build failed

@bors
Copy link
Contributor

bors commented Jan 5, 2022

☔ The latest upstream changes (presumably #92560) made this pull request unmergeable. Please resolve the merge conflicts.

@Milo123459 Milo123459 closed this Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants