Skip to content

Conversation

@Keith-Cancel
Copy link

@Keith-Cancel Keith-Cancel commented Nov 19, 2025

Experimental changes to DelimSpan::from_single() to have potential better open and close values.

Here is the tests output updated as you asked as a separate PR as asked here: #149052 (comment)

r? @petrochenkov

…est to see how the dianostics change.

I would say a lot these are cause by using DelimSpan::open or DelimSpan::close instead of Delimspan::entire when looking at the diffs.
@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 Nov 19, 2025
@tgross35
Copy link
Contributor

Could you make the PR description more meaningful? We shouldn't have to refer to a separate PR to know what this does, and it definitely does more than just update tests.

Please also make sure to squash before merge.

@Keith-Cancel Keith-Cancel changed the title Output UI test for PR #149052 DelimSpan::from_single() experiment and UI tests update. Nov 19, 2025
@Keith-Cancel
Copy link
Author

Could you make the PR description more meaningful? We shouldn't have to refer to a separate PR to know what this does, and it definitely does more than just update tests.

Please also make sure to squash before merge.

@tgross35 I updated the title of the PR, @petrochenkov wanted a separate PR for these experiments so that's what I did. I was not sure how much detail or what should be specified in such a PR. So my bad, if there is anything else please let me know.

@Keith-Cancel Keith-Cancel changed the title DelimSpan::from_single() experiment and UI tests update. [EXPERIMENT] DelimSpan::from_single() experiment and UI tests update. Nov 19, 2025
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Nov 19, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Nov 19, 2025
let pos = len.checked_sub(last.len_utf8()).unwrap_or(0);
let close = sp.subspan(pos..).unwrap_or(default_close);

Ok(match (first, last) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why the logic above is so overcomplicated.
let first = src[0] and let last = src[src.len() - 1].
open is sp.with_lo(sp.lo() + BytePos(1)) if the source map check passes, close is sp.with_hi(sp.hi() - BytePos(1)) .

@petrochenkov
Copy link
Contributor

I would say a lot the differences on those tests that changed are caused by using DelimSpan::open or Delimnspan::close instead of DelimSpand::entire() somewhere when generating the diagnostic output.

This needs to be confirmed, could you check what happens at least in the case of tests/ui/did_you_mean/bad-assoc-expr.rs? Because in that case it's a clear regression and incorrect behavior.

They also all seem to involve macro metavariables.

Of course, Delimiter::Invisible means a metavariable.
That's why I was skeptical about using shrink_to_lo/shrink_to_hi instead of the whole span in practice.

@petrochenkov petrochenkov 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 Nov 20, 2025
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-clippy Relevant to the Clippy team. 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.

5 participants