Skip to content

Conversation

JhonnyBillM
Copy link
Contributor

Addressed this suggestion #101558 (comment).

This way we comply with the Coherence rule given that IntoDiagnostic trait is defined in rustc_errors, and almost all other crates depend on it.

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 5, 2022
@rust-highfive
Copy link
Contributor

r? @eholk

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

@rustbot
Copy link
Collaborator

rustbot commented Oct 5, 2022

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 5, 2022
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

Could you try this as an alternative approach?

@eholk
Copy link
Contributor

eholk commented Oct 5, 2022

@bors r? @davidtwco

@davidtwco - You seem to have more context here so I'll let you handle this PR.

@rust-highfive rust-highfive assigned davidtwco and unassigned eholk Oct 5, 2022
@JhonnyBillM JhonnyBillM requested a review from davidtwco October 9, 2022 16:00
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

LGTM, one comment to resolve then r=me

@JhonnyBillM JhonnyBillM force-pushed the delete-target-data-layout-errors-wrapper branch from 2dc8370 to 9052800 Compare October 12, 2022 20:53
…errors

This way we comply with the Coherence rule given that IntoDiagnostic trait is defined in rustc_errors, and almost all other crates depend on it.
@JhonnyBillM JhonnyBillM force-pushed the delete-target-data-layout-errors-wrapper branch from 9052800 to be22157 Compare October 12, 2022 21:09
@JhonnyBillM JhonnyBillM requested a review from davidtwco October 12, 2022 22:11
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 13, 2022

📌 Commit be22157 has been approved by davidtwco

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 Oct 13, 2022
@bors
Copy link
Collaborator

bors commented Oct 14, 2022

⌛ Testing commit be22157 with merge 1755c85...

@bors
Copy link
Collaborator

bors commented Oct 14, 2022

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing 1755c85 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 14, 2022
@bors bors merged commit 1755c85 into rust-lang:master Oct 14, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 14, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1755c85): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
2.6% [2.4%, 2.9%] 2
Regressions ❌
(secondary)
2.4% [1.1%, 3.2%] 23
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.6% [2.4%, 2.9%] 2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)
4.3% [4.3%, 4.3%] 1
Improvements ✅
(primary)
-2.7% [-4.0%, -1.4%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.0% [-4.0%, 2.3%] 3

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@JhonnyBillM JhonnyBillM deleted the delete-target-data-layout-errors-wrapper branch October 14, 2022 13:11
@davidtwco davidtwco mentioned this pull request Nov 8, 2022
84 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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-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.

7 participants