Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
r=me |
|
but travis is unhappy |
|
Oops. Well I suppose this is what I was asking for by putting that panic in there Will fix |
|
@nikomatsakis by the way, this PR does seem very susceptible to conflicts, in the sense that any PR that adds a new Any objection to me throwing on a p=10 after I placate Travis? |
284c934 to
a49d7d5
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Oh, I guess the mode changes must apply to Okay well I'll just fix those as well. |
25fd4ab to
e829515
Compare
nikomatsakis
left a comment
There was a problem hiding this comment.
Well, this feels hacky, but I guess that was the point. compile-test definitely needs an owner to clean up and simplify the model.
src/tools/compiletest/src/runtest.rs
Outdated
There was a problem hiding this comment.
I'm not crazy about having RunPass be a distinct member of this enum, rather than just having it set flags on self.props (and/or adding appropriate flags to all the tests). e.g.,, does this mean that skip_codegen .. doesn't work on // run-pass UI tests, but does work on run-pass mode tests?
There was a problem hiding this comment.
I've been assuming that self.props was meant to reflect the actual set of directives on the header.
I can certainly change this code so that // skip-codegen works in UI tests too.
But yes, I would personally prefer if things were rearchitected so that the set of properties were established up front, based on a combination of the header (under current revision/compare-mode), and then queries could look solely at those properties.
|
I think the one thing that would be nice is some kind of comment on the mode enum explaining the special role of "run-pass", but otherwise r=me and p=10 makes sense. |
e829515 to
180ae2e
Compare
|
@bors r=nikomatsakis p=10 (Setting high priority because any new run-pass test will cause this to PR to fail in its own testing.) |
|
📌 Commit 180ae2e8f3ff78eecbf704d15f586f38bf896242 has been approved by |
|
@bors r=nikomatsakis p=10 (wait, I remembered there's a chance that the newly added run-pass test(s) will not cause any compiler diagnostics to be emitted. So re-r+'ing until I double-check locally (after my local build finishes).) |
|
📌 Commit 180ae2e8f3ff78eecbf704d15f586f38bf896242 has been approved by |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit 180ae2e8f3ff78eecbf704d15f586f38bf896242 has been approved by |
|
⌛ Testing commit 180ae2e8f3ff78eecbf704d15f586f38bf896242 with merge 33f03528ea9deb4e093fb14b9b472a21530389e1... |
* In the case of `derive-same-struct`, it seemed cleaner to add the output than to try to modify the macro itself (which is where the output is coming from). * In the case of `custom-derive-partial-eq`, it was just easier to add the output than to attempt to port the test to use a procedural macro.
b5af1d7 to
a79db05
Compare
|
@bors r=nikomatsakis p=10 |
|
📋 Looks like this PR is still in progress, ignoring approval. Hint: Remove [WIP] from this PR's title when it is ready for review. |
|
@bors r=nikomatsakis p=10 |
|
📌 Commit a79db05 has been approved by |
|
@bors p=99 rollup fairness |
|
⌛ Testing commit a79db05 with merge e43129b9cc33abdc3ddf1517dfb4080cb57a339d... |
|
💔 Test failed - status-appveyor |
|
@bors retry |
…sakis `ui`ify run-pass This addresses the remainder of #53764 by first converting `src/test/run-pass` into another `ui`-style test suite, and then turning on `compare-mode=nll` for that new suite. After this lands, we can address #54047 for the short term by moving all the `src/test/ui/run-pass` tests back to `src/test/run-pass`. (Longer term, the compiler team's current (hypothetical sketch of a) plan (see [1][], [2][]) is unify all the tests by embedding these meta-properties like "// run-pass` into their headers explicitly and dropping the significance of their location on the file system.) [1]: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/subject/weekly.20meeting.202018-09-13/near/133889370 [2]: #54047 (comment)
|
☀️ Test successful - status-appveyor, status-travis |
This addresses the remainder of #53764 by first converting
src/test/run-passinto anotherui-style test suite, and then turning oncompare-mode=nllfor that new suite.After this lands, we can address #54047 for the short term by moving all the
src/test/ui/run-passtests back tosrc/test/run-pass.(Longer term, the compiler team's current (hypothetical sketch of a) plan (see 1, 2) is unify all the tests by embedding these meta-properties like "// run-pass` into their headers explicitly and dropping the significance of their location on the file system.)