Skip to content

Suggest only Span without source changes when source code is unavailable #144585

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xizheyin
Copy link
Contributor

@xizheyin xizheyin commented Jul 28, 2025

In order to be able to reproduce in ui tests the errors that rustc generates suggestions in std sources (essentially external macros), i.e. #142403, this PR make emit_suggestion_default suggest only the base information when source is not available and does not show the code change.

Now it's something like

note: required by an implicit `Sized` bound in `Option`
--> $SRC_DIR/core/src/option.rs:LL:COL

cc #139316 (comment)

r? @jieyouxu

@rustbot
Copy link
Collaborator

rustbot commented Jul 28, 2025

jieyouxu is currently at their maximum review capacity.
They may take a while to respond.

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 28, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@xizheyin
Copy link
Contributor Author

Looks like jieyou is busy

r? compiler

@jieyouxu
Copy link
Member

jieyouxu commented Jul 28, 2025

This feels very sus. How does this handle stable vs beta vs nightly vs in-tree std sources?

I feel like there's a reason why ui tests don't get to access std sources, which is to prevent different diagnostics depending on availability of std sources, and which std sources?

@xizheyin
Copy link
Contributor Author

xizheyin commented Jul 28, 2025

How does this handle stable vs beta vs nightly vs in-tree std sources?

Is it possible to normalize the content of the source code and just let us know that the source code is displayed in stderr?

I'm curious what kind of impact different versions (stable/nightly/beta/in-tree) would have, I don't know enough about CI. Any examples of this?

@xizheyin
Copy link
Contributor Author

xizheyin commented Jul 28, 2025

Path remapped is introduced in #105500 may cc @oli-obk
Indeed it's not that simple, some of the existing test surprisingly shows something like

note: required by a bound in `Option`
  --> $SRC_DIR/core/src/option.rs:LL:COL

, just without the source code...

@xizheyin xizheyin marked this pull request as draft July 28, 2025 15:49
@rustbot rustbot 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 Jul 28, 2025
@xizheyin xizheyin changed the title Add compiletest directive show-std-source Suggest only Span without source changes when source code is unavailable Jul 28, 2025
Comment on lines +25 to +27
help: consider dereferencing here
--> $SRC_DIR/core/src/macros/mod.rs:LL:COL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've written up this change in the PR description above, and it should have a similar mechanism to span_note for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should be suggesting changes to code the user doesn't own, specially for stdlib. For a suggestion here it should be pointing at the parameters instead (which might be difficult to accomplish).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is exactly the problem we need to solve.

This pr is to solve the problem that UI test can't be reproduced to make suggestions in std. Being able to reproduce compiler bugs will make review easier.

This comment was marked as duplicate.

@xizheyin xizheyin marked this pull request as ready for review July 28, 2025 17:44
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 28, 2025
@xizheyin
Copy link
Contributor Author

For further explanation, I moved the comments here.

When fixing the problems listed in #142403, we found that the bug could not be reproduced in ui test. After investigation, I found that span suggestion and span note are not realized in the same way, so I submitted this pr to make std visible in ui tests.

In daily use, the source of std should be visible, and execution will fall to the else branch, so these will not be displayed.

This new test is a bug recorded in #142403, only for verifying this code change, I will fix it in later pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

6 participants