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

Fix find_format_arg_expr when incremental compilation is enabled #10980

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

Alexendoo
Copy link
Member

Fixes #10969

lower_span gives AST spans a parent when lowering to the HIR. That meant the == span comparison would return false even though the spans are pointing to the same thing. We now ignore the parent when comparing the spans

Debugging this was quite the puzzle, because the parent is not included in the debug output of Spans or SpanData 😬

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Jun 17, 2023

r? @dswij

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 17, 2023
@Alexendoo Alexendoo force-pushed the format-args-incremental-spans branch from 17f8fdd to c4be901 Compare June 17, 2023 20:54
@Alexendoo Alexendoo force-pushed the format-args-incremental-spans branch from c4be901 to 26e78e7 Compare June 17, 2023 20:55
Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this!

Comment on lines +426 to +427
// When incremental compilation is enabled spans gain a parent during AST to HIR lowering,
// since we're comparing an AST span to a HIR one we need to ignore the parent field
Copy link
Member

Choose a reason for hiding this comment

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

Huh, TIL 👀

@dswij
Copy link
Member

dswij commented Jun 19, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jun 19, 2023

📌 Commit 26e78e7 has been approved by dswij

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 19, 2023

⌛ Testing commit 26e78e7 with merge ebcbba9...

@bors
Copy link
Contributor

bors commented Jun 19, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing ebcbba9 to master...

@bors bors merged commit ebcbba9 into rust-lang:master Jun 19, 2023
@Alexendoo Alexendoo deleted the format-args-incremental-spans branch June 19, 2023 18:20
bors added a commit that referenced this pull request Sep 12, 2023
Ignore span's parents in `collect_ast_format_args`/`find_format_args`

Fixes #11470, covers some cases missed by #10980

Can't have a test yet because of #11126 but it works locally

changelog: none

r? `@dswij`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

to_string_in_format_args not caught on nightly without --release
4 participants