modify mir-opt testing infrastructure to require that lines appear continuously #45153
Labels
E-easy
Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
E-mentor
Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
nikomatsakis
added
E-easy
|
cc @chrisvittal, who expressed some interest in this |
23 tasks
|
This should be straightforward. I'd love to take it on. |
chrisvittal
added a commit
to chrisvittal/rust
that referenced
this issue
Oct 10, 2017
Mir testing now requires that lines be continuous. To achive this,
instead of collecting the expected mir as a string, it is now wrapped in
an `ExpectedLine` enum, that is either `Elision` or `Text(T)` where `T:
AsRef<str>`. `Text` lines must be matched in order, unless separated by
`Elision` lines. Matches occur greedily, that is, an Elision will skip
as few lines as possible.
To add a new elision marker. Put a comment containing only "..." and
whitespace in any MIR testing block. Like so:
```
// fn write_42(_1: *mut i32) -> bool {
// ...
// bb0: {
// Validate(Acquire, [_1: *mut i32]);
// Validate(Release, [_1: *mut i32]);
// ...
// return;
// }
// }
```
Right now, all input before the line right after `// START` is elided,
and all input after the line right before `// END` is also not tested.
Many tests need to be updated. That will follow in the next commit.
cc rust-lang#45153
bors
added a commit
that referenced
this issue
Oct 14, 2017
Modify MIR testing to require consecutive lines
MIR testing now requires that lines be consecutive. To achive this,
instead of collecting the expected mir as a string, it is now wrapped in
an `ExpectedLine` enum, that is either `Elision` or `Text(T)` where `T:
AsRef<str>`. `Text` lines must be matched in order, unless separated by
`Elision` lines. Elision occurs lazily, that is, an Elision will skip
as few lines as possible.
To add a new elision marker. Put a comment containing only "..." and
whitespace in any MIR testing block. Like so:
```
// fn write_42(_1: *mut i32) -> bool {
// ...
// bb0: {
// Validate(Acquire, [_1: *mut i32]);
// Validate(Release, [_1: *mut i32]);
// ...
// return;
// }
// }
```
Right now, all input before the line right after `// START` is elided,
and all input after the line right before `// END` is also not tested.
Many tests need to be updated. That will follow in the next commit.
cc #45153
r? @nikomatsakis
|
I believe this is resolved by #45162 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The mir-opt testing infrastructure (described here) allows us to write tests that do various "string matches" against the MIR output. The idea is that you can add comments at the end of a test file like this one that will specify output that must appear in the "dump-mir" output. This is an example of such a comment.
The basic structure is like this:
The test infrastructure will then check that (a)
line0appears in the given mir file and (b)line1appears sometime thereafter. However, it also allows for random stuff to appear in the middle. This is intended to make the test robust, but it also makes things quite surprising -- if you look at the tests, you'll see we mostly don't really want that most of the time.What I think we should do is modify it so that
line0andline1must appear consecutively. For bonus points, we could allow a special comment like// ...to be included that would indicate that zero or more lines may be skipped in between.The code that implements this check can be found in in
runtest.rs. It begins here, with this loop that walks over the.rstest file, extracting the expected lines. The expected lines are accumulated into a vector. Whenever we reach a// ENDcomment, we then invokecompare_mir_test_output(), which will open the relevant file and search for the expected lines.So the idea would be to detect
// ...and record that in the vector (e.g., we might store aVec<ExpectedLine>, whereenum ExpectedLine { Ellipsis, Some(String) }). Incompare_mir_test_output(), we would then skip lines only when anEllipsisis found.We will possibly have to adjust some of the existing tests, I'm not sure, but we should be able to do so readily enough.
The text was updated successfully, but these errors were encountered: