-
Notifications
You must be signed in to change notification settings - Fork 3
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
passes: Add pass to run gccrs on the full rustc ui testsuite #7
Conversation
fe2d4ee
to
9891ebe
Compare
9891ebe
to
6727c86
Compare
This pass is actually three new passes: A regular one, one with only the files successfully compiling with `#![no_std]` and one with only the files succesfully compiling with `#![no_core]`
Co-authored-by: flip1995 <philipp.krones@embecosm.com>
src/passes/gccrs_rustc_successes.rs
Outdated
|
||
// We're only interested in successes | ||
if test_content.contains("ERROR") { | ||
return Ok(TestCase::default()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC you return an empty TestCase
as a kind of an error state. Why not add another Error
variant and return that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind returning an "empty" test case and then checking for it afterwards (in your next comment) is that it is still valid - we simply want to skip this file and not generate a proper assertion for it. try_fold
short circuits the execution on Err
values, so we would end up not generating all the test cases we want. Unless I'm misunderstanding something about the way try_fold
works, which is entirely possible :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the naming and checking afterwards could be better. I'll work on something!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm..
Maybe turn the TestCase
struct into an enum TestCase { Test{..}, Skip }
and then return TestCase::Skip
if you want to skip it.
src/main.rs
Outdated
if test_case.is_empty() { | ||
Ok(acc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When returning a proper error instead of a returning and implicit tristate type, this change is unnecessary.
@flip1995 I turned |
289c336
to
ba2af10
Compare
Co-authored-by: flip1995 <hello@philkrones.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (you forgot to merge :))
What we now need is a translator from rustc errors to gccrs errors, and a fallback invalid error message in case the error does not exist/cannot be represented by gccrs yet, causing the test to assurely fail.
This will also require you to update ftf to benefit from the new matching behavior on stdout/stderr instead of strict equality