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

When displaying multispans, ignore empty lines adjacent to ... #122029

Merged
merged 4 commits into from
Mar 20, 2024

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 5, 2024

error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:98:18
    |
6   |       let _ = match true {
    |               ---------- `match` arms have incompatible types
7   |           true => (
    |  _________________-
8   | |             // last line shown in multispan header
...   |
96  | |
97  | |         ),
    | |_________- this is found to be of type `()`
98  |           false => "
    |  __________________^
...   |
119 | |
120 | |         ",
    | |_________^ expected `()`, found `&str`

error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:215:18
    |
122 |       let _ = match true {
    |               ---------- `match` arms have incompatible types
123 |           true => (
    |  _________________-
124 | |
125 | |         1 // last line shown in multispan header
...   |
213 | |
214 | |         ),
    | |_________- this is found to be of type `{integer}`
215 |           false => "
    |  __________________^
216 | |
217 | |
218 | |         1 last line shown in multispan
...   |
237 | |
238 | |         ",
    | |_________^ expected integer, found `&str`

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2024

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 5, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

r=me after merging the fixup commits into their corresponding main commits

compiler/rustc_errors/src/emitter.rs Outdated Show resolved Hide resolved
@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 Mar 5, 2024
@bors
Copy link
Contributor

bors commented Mar 8, 2024

☔ The latest upstream changes (presumably #121500) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank
Copy link
Contributor Author

estebank commented Mar 8, 2024

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Mar 8, 2024

📌 Commit e119b32 has been approved by oli-obk

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

@bors rollup

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 9, 2024
When displaying multispans, ignore empty lines adjacent to `...`

```
error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:98:18
    |
6   |       let _ = match true {
    |               ---------- `match` arms have incompatible types
7   |           true => (
    |  _________________-
8   | |             // last line shown in multispan header
...   |
96  | |
97  | |         ),
    | |_________- this is found to be of type `()`
98  |           false => "
    |  __________________^
...   |
119 | |
120 | |         ",
    | |_________^ expected `()`, found `&str`

error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:215:18
    |
122 |       let _ = match true {
    |               ---------- `match` arms have incompatible types
123 |           true => (
    |  _________________-
124 | |
125 | |         1 // last line shown in multispan header
...   |
213 | |
214 | |         ),
    | |_________- this is found to be of type `{integer}`
215 |           false => "
    |  __________________^
216 | |
217 | |
218 | |         1 last line shown in multispan
...   |
237 | |
238 | |         ",
    | |_________^ expected integer, found `&str`
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#121561 (Detect typos for compiletest test directives)
 - rust-lang#121567 (Avoid some interning in bootstrap)
 - rust-lang#121685 (Fixing shellcheck comments on lvi test script)
 - rust-lang#121833 (Suggest correct path in include_bytes!)
 - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once)
 - rust-lang#121907 (skip sanity check for non-host targets in `check` builds)
 - rust-lang#122029 (When displaying multispans, ignore empty lines adjacent to `...`)
 - rust-lang#122221 (match lowering: define a convenient struct)
 - rust-lang#122244 (fix: LocalWaker memory leak and some stability attributes)
 - rust-lang#122251 (Add test to check unused_lifetimes don't duplicate "parameter is never used" error)

r? `@ghost`
`@rustbot` modify labels: rollup
@workingjubilee
Copy link
Member

@bors r-

Seems to have busted rollup in #122260 (comment)

@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 Mar 10, 2024
@workingjubilee
Copy link
Member

@bors rollup=iffy

@estebank
Copy link
Contributor Author

Oooff... Seeing what the diff was concerns me about relying on the svg output going forward :-/

@estebank
Copy link
Contributor Author

@epage could you bring some insight on what the reason might be for this divergence in the output?

---- [ui] tests/ui/codemap_tests/huge_multispan_highlight.rs stdout ----
diff of svg:

- <svg width="743px" height="848px" xmlns="http://www.w3.org/2000/svg">
+ <svg width="740px" height="848px" xmlns="http://www.w3.org/2000/svg">
2   <style>
3     .fg { fill: #AAAAAA }
4     .bg { background: #000000 }

I suspected maybe font size differences causing some calculation to diverge?

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 18, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@estebank
Copy link
Contributor Author

@epage that's a shame. For now I think that setting a min-width unblocks us, but we should be aware of that issue. For humans it doesn't matter, but it will get in the way of tooling doing diffs (like the cargo test suite, also surprised it didn't happen there either).

@epage
Copy link
Contributor

epage commented Mar 18, 2024

Cargo is getting this functionality through snapbox which has a (hacky) solution to only pay attention to the <text> and not to the stuff at the top so it gives a more focused diff to the user.

@estebank
Copy link
Contributor Author

@epage ah! I see. That makes sense, to make the differ smarter and ignore what humans would ignore. For now I'll go with the min-width approach to see if that's enough, so that we can still detect changes in the output that would cause our bounding box to change, but if it continues to give us trouble I'll modify the diff checker to ignore the sizing info (shouldn't be too hard to do, it can even be "ignore first line").

@estebank
Copy link
Contributor Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Mar 18, 2024

📌 Commit 957c0d3 has been approved by oli-obk

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

bors commented Mar 18, 2024

⌛ Testing commit 957c0d3 with merge f601f46...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2024
When displaying multispans, ignore empty lines adjacent to `...`

```
error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:98:18
    |
6   |       let _ = match true {
    |               ---------- `match` arms have incompatible types
7   |           true => (
    |  _________________-
8   | |             // last line shown in multispan header
...   |
96  | |
97  | |         ),
    | |_________- this is found to be of type `()`
98  |           false => "
    |  __________________^
...   |
119 | |
120 | |         ",
    | |_________^ expected `()`, found `&str`

error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:215:18
    |
122 |       let _ = match true {
    |               ---------- `match` arms have incompatible types
123 |           true => (
    |  _________________-
124 | |
125 | |         1 // last line shown in multispan header
...   |
213 | |
214 | |         ),
    | |_________- this is found to be of type `{integer}`
215 |           false => "
    |  __________________^
216 | |
217 | |
218 | |         1 last line shown in multispan
...   |
237 | |
238 | |         ",
    | |_________^ expected integer, found `&str`
```
@bors
Copy link
Contributor

bors commented Mar 19, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 19, 2024
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] rustc_infer test:false 92.205
   Compiling rustc_passes v0.0.0 (/checkout/compiler/rustc_passes)
[RUSTC-TIMING] rustc_traits test:false 20.811
   Compiling rustc_mir_transform v0.0.0 (/checkout/compiler/rustc_mir_transform)
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
##[group]Clock drift check
  local time: Tue Mar 19 00:02:06 UTC 2024
  network time: Tue, 19 Mar 2024 00:02:06 GMT
##[endgroup]
##[endgroup]
Session terminated, killing shell...[RUSTC-TIMING] rustc_middle test:false 130.289
 ...killed.
##[error]The operation was canceled.

@estebank
Copy link
Contributor Author

@bors retry

@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 Mar 19, 2024
@bors
Copy link
Contributor

bors commented Mar 19, 2024

⌛ Testing commit 957c0d3 with merge bd459c2...

@bors
Copy link
Contributor

bors commented Mar 20, 2024

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing bd459c2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 20, 2024
@bors bors merged commit bd459c2 into rust-lang:master Mar 20, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 20, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (bd459c2): 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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.3% [-5.1%, -1.9%] 38
All ❌✅ (primary) - - 0

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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% [-2.7%, -2.7%] 1
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 668.419s -> 668.413s (-0.00%)
Artifact size: 312.82 MiB -> 312.83 MiB (0.00%)

@nikic
Copy link
Contributor

nikic commented Mar 20, 2024

The test still fails for me locally:

---- [ui] tests/ui/codemap_tests/huge_multispan_highlight.rs stdout ----
diff of svg:

-	<svg width="750px" height="848px" xmlns="http://www.w3.org/2000/svg">
+	<svg width="751px" height="848px" xmlns="http://www.w3.org/2000/svg">
2	  <style>
3	    .fg { fill: #AAAAAA }
4	    .bg { background: #000000 }

@estebank
Copy link
Contributor Author

estebank commented Mar 20, 2024

@nikic I'll make an emergency PR to completely ignore the width then :-/

Edit: fix #122779

matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this pull request Mar 21, 2024
When displaying multispans, ignore empty lines adjacent to `...`

```
error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:98:18
    |
6   |       let _ = match true {
    |               ---------- `match` arms have incompatible types
7   |           true => (
    |  _________________-
8   | |             // last line shown in multispan header
...   |
96  | |
97  | |         ),
    | |_________- this is found to be of type `()`
98  |           false => "
    |  __________________^
...   |
119 | |
120 | |         ",
    | |_________^ expected `()`, found `&str`

error[E0308]: `match` arms have incompatible types
   --> tests/ui/codemap_tests/huge_multispan_highlight.rs:215:18
    |
122 |       let _ = match true {
    |               ---------- `match` arms have incompatible types
123 |           true => (
    |  _________________-
124 | |
125 | |         1 // last line shown in multispan header
...   |
213 | |
214 | |         ),
    | |_________- this is found to be of type `{integer}`
215 |           false => "
    |  __________________^
216 | |
217 | |
218 | |         1 last line shown in multispan
...   |
237 | |
238 | |         ",
    | |_________^ expected integer, found `&str`
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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-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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.