Skip to content

Conversation

@Firestar99
Copy link
Member

@Firestar99 Firestar99 commented Oct 21, 2025

Supersedes #439, see discussions over there

  • pass -Zui-testing to rustc to improve compiletest normalization and have fewer conflicts
  • review commit by commit

@nnethercote
Copy link
Contributor

I would squash together the 2nd and 3rd commits because they don't make sense in isolation (tests will fail if just the 2nd commit is applied). Otherwise, I'm not a rust-gpu reviewer, but LGTM.

@Firestar99
Copy link
Member Author

I was considering it, but didn't as it "hides" the "-Zui-testing flag added" change. Whatever, let's squash them

@Firestar99 Firestar99 force-pushed the compiletest-z-ui-testing branch from 18c92d8 to 067b38b Compare October 21, 2025 08:27
@Firestar99 Firestar99 enabled auto-merge October 21, 2025 08:28
@Firestar99 Firestar99 mentioned this pull request Oct 21, 2025
@nnethercote
Copy link
Contributor

"hides" the "-Zui-testing flag added" change

It's mentioned in the first line of the commit message! :)

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.

Sweet! Thanks for doing this.

@Firestar99 Firestar99 added this pull request to the merge queue Oct 22, 2025
LegNeato added a commit to LegNeato/Rust-CUDA that referenced this pull request Oct 22, 2025
Merged via the queue into main with commit 300da9c Oct 22, 2025
13 checks passed
@Firestar99 Firestar99 deleted the compiletest-z-ui-testing branch October 22, 2025 04:24
LegNeato added a commit to Rust-GPU/rust-cuda that referenced this pull request Oct 22, 2025
Copy link
Member

@eddyb eddyb left a comment

Choose a reason for hiding this comment

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

To be perfectly honest, seeing this diff, the fact that we weren't using -Z ui-testing for the kind of tests named ui because of rustc testing infrastructure, is quite the facepalm moment 🤦.

Thanks for cleaning this up!

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.

4 participants