Skip to content

Tweak await span to not contain dot #110823

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
Merged
merged 6 commits into from
May 1, 2023
Merged

Tweak await span to not contain dot #110823

merged 6 commits into from
May 1, 2023

Conversation

compiler-errors
Copy link
Member

Fixes a discrepancy between method calls and await expressions where the latter are desugared to have a span that contains the dot (i.e. .await) but method call identifiers don't contain the dot. This leads to weird suggestions suggestions in borrowck -- see linked issue.

Fixes #110761

This mostly touches a bunch of tests to tighten their await span.

@rustbot
Copy link
Collaborator

rustbot commented Apr 25, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added 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 Apr 25, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 25, 2023

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Apr 25, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
b-naber
b-naber reviewed Apr 28, 2023
View reviewed changes
compiler/rustc_middle/src/util/call_kind.rs
@@ -129,6 +132,8 @@ pub fn call_kind<'tcx>(
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::TryBlock)
{
Some((CallDesugaringKind::TryBlockFromOutput, method_substs.type_at(0)))
} else if fn_call_span.is_desugaring(DesugaringKind::Await) {
Some((CallDesugaringKind::Await, method_substs.type_at(0)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems somewhat unexpected to have this under CallKind::Normal. The docs say CallKind::Normal corresponds to method calls. I mean this is consistent with previous usage, but maybe we should introduce something like CallKind::Other?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... this just feels like uplifting the desugaring part of the Normal variant up to its own variant. I don't see how this makes things much cleaner 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I just meant that this would be more consistent with the docs of CallKind::Normal. Feel free to ignore since CallKind::Normal was also used for other CallDesugaringKinds before for which that variant doesn't really match the docs afaict. The rest of the PR looks good to me, so r=me.

@compiler-errors
Copy link
Member Author

@bors r=b-naber

@bors
Copy link
Collaborator

bors commented Apr 30, 2023

📌 Commit 6d6c904 has been approved by b-naber

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 Apr 30, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 30, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 1, 2023
@bors
Auto merge of rust-lang#111040 - matthiaskrgr:rollup-g2sns0f, r=matth…
2034b6d 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110823 (Tweak await span to not contain dot)
 - rust-lang#111015 (Remove wrong assertion in match checking.)
 - rust-lang#111023 (Test precise capture with a multi-variant enum and exhaustive patterns)
 - rust-lang#111032 (Migrate `builtin_macros::asm` diagnostics to translatable diagnostics)
 - rust-lang#111033 (Ping Nadrieril when changing exhaustiveness checking)
 - rust-lang#111037 (Close parentheses for `offset_of` in AST pretty printing)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1b262b8 into rust-lang:master May 1, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 1, 2023
@compiler-errors compiler-errors deleted the tweak-await-span branch August 11, 2023 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjgillot cjgillot cjgillot left review comments

@calebcartwright calebcartwright calebcartwright left review comments

@b-naber b-naber b-naber left review comments

Assignees

@b-naber b-naber

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.71.0
Development

Successfully merging this pull request may close these issues.

Typo in error output: Instead of clone()..await it should be .clone().await
7 participants
@compiler-errors @rustbot @rust-log-analyzer @bors @cjgillot @calebcartwright @b-naber