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 diagnostic items #96302

Merged
merged 2 commits into from
May 8, 2022
Merged

Conversation

Serial-ATA
Copy link
Contributor

This just adds a handful diagnostic items I noticed were missing.

Would it be worth doing this for all of the remaining types? I'm willing to do it if it'd be helpful.

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 22, 2022
@rust-highfive
Copy link
Collaborator

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 r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. 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-highfive
Copy link
Collaborator

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 22, 2022
@rust-log-analyzer

This comment has been minimized.

@Serial-ATA
Copy link
Contributor Author

Serial-ATA commented Apr 22, 2022

I think it's hitting this:

if self.tcx.trait_is_auto(def_id)
|| self.tcx.lang_items().items().contains(&Some(def_id))
|| self.tcx.get_diagnostic_name(def_id).is_some()
{
// Mentioning implementers of `Copy`, `Debug` and friends is not useful.
return false;
}

Should a special case be added for Error? Actually, why is it even hitting that? It's cfg(not(test)).

@Serial-ATA
Copy link
Contributor Author

r? @Manishearth

@rust-highfive rust-highfive assigned Manishearth and unassigned kennytm Apr 22, 2022
@Manishearth
Copy link
Member

Manishearth commented Apr 25, 2022

@bors r+

seems fine, unsure if we need to add more

cc @estebank just to keep him aware

@bors
Copy link
Contributor

bors commented Apr 25, 2022

📌 Commit ea013e0 has been approved by Manishearth

@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 25, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 25, 2022
…=Manishearth

Add more diagnostic items

This just adds a handful diagnostic items I noticed were missing.

Would it be worth doing this for all of the remaining types? I'm willing to do it if it'd be helpful.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 25, 2022
…=Manishearth

Add more diagnostic items

This just adds a handful diagnostic items I noticed were missing.

Would it be worth doing this for all of the remaining types? I'm willing to do it if it'd be helpful.
@GuillaumeGomez
Copy link
Member

Ci isn't happy.

@bors: r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 25, 2022
@Serial-ATA
Copy link
Contributor Author

Here's why CI is failing: #96302 (comment). It shouldn't be interfering with tests, so I'm not sure what's going on here.

@Manishearth
Copy link
Member

Not sure either, @estebank?

@estebank
Copy link
Contributor

estebank commented Apr 26, 2022

Oh! So that's a very crude filter to avoid verbose output, relying on the assumption that if something is a diagnostic item there are better things than list them in those errors. Can you do ./x.py test src/test/ui --bless and add the changes to .stderr files to a new commit so we can see if the changes are acceptable?

In the past I've added diagnostic items on an as-needed basis, though (as they can be safely added in the same commit as the change that uses them).

@Serial-ATA Serial-ATA force-pushed the more-diagnostic-items branch from ea013e0 to a235c1d Compare April 26, 2022 22:02
@Serial-ATA
Copy link
Contributor Author

I'm confused. Over 700 files were modified, and most of these changes don't even look remotely related to what's been added. The only diagnostic item I needed is causing so many problems 😄

@estebank
Copy link
Contributor

Try pulling a more recent master and rebasing your changes on top of it, I'm noticing some output changes caused by a relatively recently landed PR.

@Serial-ATA Serial-ATA force-pushed the more-diagnostic-items branch 2 times, most recently from c9bf370 to a1e0cfa Compare April 27, 2022 00:25
@Serial-ATA
Copy link
Contributor Author

Oops, I blindly added COMPILETEST_FORCE_STAGE0=1 since it wouldn't let me do ./x.py test src/test/ui --bless. Turns out the only difference is that one file from before.

@estebank
Copy link
Contributor

I'm as baffled as you are 😮

@bors retry lets check it's not spurious

@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 28, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 28, 2022
…=manishearth

Add more diagnostic items

This just adds a handful diagnostic items I noticed were missing.

Would it be worth doing this for all of the remaining types? I'm willing to do it if it'd be helpful.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 28, 2022
…=manishearth

Add more diagnostic items

This just adds a handful diagnostic items I noticed were missing.

Would it be worth doing this for all of the remaining types? I'm willing to do it if it'd be helpful.
@Dylan-DPC
Copy link
Member

failed in rollup

@Dylan-DPC
Copy link
Member

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 28, 2022
@Serial-ATA Serial-ATA force-pushed the more-diagnostic-items branch from a1e0cfa to be9dd2d Compare April 28, 2022 20:56
@Serial-ATA
Copy link
Contributor Author

Looks like the failing test just had a windows only version added. Hopefully that's the last issue with this 😄

@estebank
Copy link
Contributor

@bors r=manishearth rollup=never

@xFrednet
Copy link
Member

xFrednet commented May 8, 2022

I'm guessing bors is stuck

@bors ping

@bors
Copy link
Contributor

bors commented May 8, 2022

😪 I'm awake I'm awake

@xFrednet
Copy link
Member

xFrednet commented May 8, 2022

Copied from this previous comment. I hope that's okay 😅

@bors r=manishearth rollup=never

@bors
Copy link
Contributor

bors commented May 8, 2022

📌 Commit be9dd2d has been approved by manishearth

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 8, 2022
@bors
Copy link
Contributor

bors commented May 8, 2022

⌛ Testing commit be9dd2d with merge 6846164...

@bors
Copy link
Contributor

bors commented May 8, 2022

☀️ Test successful - checks-actions
Approved by: manishearth
Pushing 6846164 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 8, 2022
@bors bors merged commit 6846164 into rust-lang:master May 8, 2022
@rustbot rustbot added this to the 1.62.0 milestone May 8, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6846164): comparison url.

Summary:

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvement found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 0 0 0 1 0
mean2 N/A N/A N/A -1.4% N/A
max N/A N/A N/A -1.4% N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.