Skip to content

Conversation

Firestar99
Copy link
Member

@Firestar99 Firestar99 commented Oct 14, 2025

Requires #380

To reduce compiletest stderr merge conflicts and reviewing effort:

  • Removes line and column from rustc errors:
    * `// normalize-stderr-test "\.rs:\d+:\d+" -> ".rs:""` remove the line and column from error messages, eg. `--> path/to/some.rs:8:6`
    * `// normalize-stderr-test "(\n)\d* *([ -])([\|\+\-\=])" -> "$1 $2$3""` remove the line number prefixing the source code snippet, does not touch `...`
  • Removes OpLine from some tests

@Firestar99 Firestar99 force-pushed the compiletest-normalization branch from bbb1919 to 065b526 Compare October 14, 2025 13:24
Base automatically changed from vec3-12-bytes to main October 15, 2025 10:09
@Firestar99 Firestar99 force-pushed the compiletest-normalization branch from 065b526 to 121a661 Compare October 16, 2025 09:26
@Firestar99 Firestar99 marked this pull request as ready for review October 16, 2025 14:24
Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

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

Doesn't appear to be a way we can register this as a default with compiletest and just have a directive like include-file-numbers to turn them on when needed. But there is -Z ui-testing, perhaps we can set that in compiletests's Config https://github.com/Manishearth/compiletest-rs/blob/02b122626320f562914710bada39e637f051c7bb/src/common.rs#L172? do we ever care about the line numbers?

It looks like his might be possible / better in the newer rustc compiletest framework, but I had other issues with it: https://rustc-dev-guide.rust-lang.org/tests/ui.html

@Firestar99
Copy link
Member Author

Firestar99 commented Oct 16, 2025

-Z ui-testing is a good find! Pushed that alternative to compiletest-z-ui-testing, if you want to have a look. The main difference is that the rustc flag keeps the line numbers on the actual error, and just removes the line numbers on the code snippets. I don't really have a preference for which one to merge, feel free to decide.

@eddyb @nnethercote @FractalFir preferences?

(Unfortunately github compare between the branches doesn't actually produce a proper diff, but just shows the diff to the parent commit... so here's a manual sample diff from invalid_vector_type:)

 error: `#[spirv(vector)]` must have 2, 3 or 4 members
-   --> $DIR/invalid_vector_type.rs:
-    |
-    | pub struct FewerFields {
-    | ^^^^^^^^^^^^^^^^^^^^^^
+  --> $DIR/invalid_vector_type.rs:8:1
+   |
+LL | pub struct FewerFields {
+   | ^^^^^^^^^^^^^^^^^^^^^^
 
 error: `#[spirv(vector)]` must have 2, 3 or 4 members
-   --> $DIR/invalid_vector_type.rs:
-    |
-    | pub struct TooManyFields {
-    | ^^^^^^^^^^^^^^^^^^^^^^^^
+  --> $DIR/invalid_vector_type.rs:13:1
+   |
+LL | pub struct TooManyFields {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: `#[spirv(vector)]` type fields must all be floats, integers or bools
-   --> $DIR/invalid_vector_type.rs:
-    |
-    | pub struct NotVectorField {
-    | ^^^^^^^^^^^^^^^^^^^^^^^^^
-    |
-    = note: field type is f32x2
+  --> $DIR/invalid_vector_type.rs:22:1
+   |
+LL | pub struct NotVectorField {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: field type is f32x2
 
 error: `#[spirv(vector)]` member types must all be the same
-   --> $DIR/invalid_vector_type.rs:
-    |
-    | pub struct DifferentTypes {
-    | ^^^^^^^^^^^^^^^^^^^^^^^^^
+  --> $DIR/invalid_vector_type.rs:34:1
+   |
+LL | pub struct DifferentTypes {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 4 previous errors
 

We may need to modernize our compiletest at some point, but I don't think we should block this PR because of that. I'm mostly waiting for my difftest PR to be reviewed before I'll touch it, so I can immediately integrate the feedback from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants