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

Error reporting order on riscv64gc-unknown-linux-gnu #72913

Closed
tblah opened this issue Jun 2, 2020 · 1 comment
Closed

Error reporting order on riscv64gc-unknown-linux-gnu #72913

tblah opened this issue Jun 2, 2020 · 1 comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-reproducibility Area: Reproducible / deterministic builds C-bug Category: This is a bug. O-riscv Target: RISC-V architecture T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tblah
Copy link
Contributor

tblah commented Jun 2, 2020

I tried running ui tests for riscv64gc-unknown-linux-gnu, in particular ui/or-patterns/missing-bindings.rs. Reproduce with

./x.py test --target risv64gc-unknown-linux-gnu src/test/ui/or-patterns/missing-bindings.rs

The test checks that the expected errors are encountered when compiling missing-bindings.rs. On riscv64gc-unknown-linux-gnu the correct errors are reported but in the wrong order (see below).

I looked into the first of these out of order errors and found the errors are sorted (src/librustc_resolve/late.rs:1326) by their Symbol, which sorts by the index of that interned string. I think the problem comes because some of the variable names (a, b, c) in missing-bindings.rs are interned before the source is parsed only for riscv64gc-unknown-linux-gnu here.

One fix would be to sort the error order by the Symbol string instead of the index, but that would effect all target architectures. Edit: this can be found here.

Meta

rustc built from 10c2316

Test output

---- [ui] ui/or-patterns/missing-bindings.rs stdout ----
diff of stderr:

87	   |             |
88	   |             variable not in all patterns
89	
-	error[E0408]: variable `b` is not bound in all patterns
-	  --> $DIR/missing-bindings.rs:46:21
-	   |
-	LL |     let A(A(a, b) | B(c), d) | B(e) = Y;
-	   |                -    ^^^^ pattern doesn't bind `b`
-	   |                |
-	   |                variable not in all patterns
-	
98	error[E0408]: variable `c` is not bound in all patterns
99	  --> $DIR/missing-bindings.rs:46:11
100	   |

103	   |           |
104	   |           pattern doesn't bind `c`
105	
+	error[E0408]: variable `b` is not bound in all patterns
+	  --> $DIR/missing-bindings.rs:46:21
+	   |
+	LL |     let A(A(a, b) | B(c), d) | B(e) = Y;
+	   |                -    ^^^^ pattern doesn't bind `b`
+	   |                |
+	   |                variable not in all patterns
+	
106	error[E0408]: variable `a` is not bound in all patterns
107	  --> $DIR/missing-bindings.rs:46:32
108	   |

111	   |             |
112	   |             variable not in all patterns
113	
-	error[E0408]: variable `b` is not bound in all patterns
-	  --> $DIR/missing-bindings.rs:46:32
-	   |
-	LL |     let A(A(a, b) | B(c), d) | B(e) = Y;
-	   |                -               ^^^^ pattern doesn't bind `b`
-	   |                |
-	   |                variable not in all patterns
-	
122	error[E0408]: variable `c` is not bound in all patterns
123	  --> $DIR/missing-bindings.rs:46:32
124	   |

135	   |                           |
136	   |                           variable not in all patterns
137	
+	error[E0408]: variable `b` is not bound in all patterns
+	  --> $DIR/missing-bindings.rs:46:32
+	   |
+	LL |     let A(A(a, b) | B(c), d) | B(e) = Y;
+	   |                -               ^^^^ pattern doesn't bind `b`
+	   |                |
+	   |                variable not in all patterns
+	
138	error[E0408]: variable `e` is not bound in all patterns
139	  --> $DIR/missing-bindings.rs:46:9
140	   |

197	LL |             V3(c),
198	   |             ^^^^^ pattern doesn't bind `a`
199	
-	error[E0408]: variable `b` is not bound in all patterns
-	  --> $DIR/missing-bindings.rs:58:13
-	   |
-	LL | /             V1(
-	LL | |
-	LL | |
-	LL | |                 A(
-	...  |
-	LL | |                 B(Ok(a) | Err(a))
-	LL | |             ) |
-	   | |_____________^ pattern doesn't bind `b`
-	...
-	LL |                       B(b),
-	   |                         - variable not in all patterns
-	...
-	LL |               V3(c),
-	   |               ^^^^^ pattern doesn't bind `b`
-	
218	error[E0408]: variable `c` is not bound in all patterns
219	  --> $DIR/missing-bindings.rs:58:13
220	   |

236	   | |_____________^ pattern doesn't bind `c`
237	LL |               V3(c),
238	   |                  - variable not in all patterns
+	
+	error[E0408]: variable `b` is not bound in all patterns
+	  --> $DIR/missing-bindings.rs:58:13
+	   |
+	LL | /             V1(
+	LL | |
+	LL | |
+	LL | |                 A(
+	...  |
+	LL | |                 B(Ok(a) | Err(a))
+	LL | |             ) |
+	   | |_____________^ pattern doesn't bind `b`
+	...
+	LL |                       B(b),
+	   |                         - variable not in all patterns
+	...
+	LL |               V3(c),
+	   |               ^^^^^ pattern doesn't bind `b`
239	
240	error: aborting due to 26 previous errors
241	
@tblah tblah added the C-bug Category: This is a bug. label Jun 2, 2020
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-reproducibility Area: Reproducible / deterministic builds O-riscv Target: RISC-V architecture T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 2, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 5, 2020
resolve: Sort E0408 errors by Symbol str

This is a request for comments implementing my suggested solution to rust-lang#72913

Previously errors were sorted by Symbol index instead of the string. The indexes are not the same between architectures because Symbols for architecture extensions (e.g. x86 AVX or RISC-V d) are interned before the source file is parsed. RISC-V's naming of extensions after single letters led to it having errors sorted differently for test cases using single letter variable names. Instead sort the errors by the Symbol string so that it is stable across architectures.

While I was at it, there's also 8edb05c  skipping some ui tests which I think are irrelevant for risc-v.
@tblah
Copy link
Contributor Author

tblah commented Jun 8, 2020

Closing now that #72982 is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-reproducibility Area: Reproducible / deterministic builds C-bug Category: This is a bug. O-riscv Target: RISC-V architecture T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants