-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Move some compile-fail
tests to ui
#52409
Conversation
@@ -1,31 +0,0 @@ | |||
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT |
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 deleted files (such as this one) are moves with changes.
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.
why were they changed?
and why in a single commit? 😢
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 had to add the error note check, as they were relying only on regex matching the output. I couldn't tell in advance whether a given test would need to be changed before running them as ui
. I can rewrite the commit history to do so later.
r? @oli-obk |
This comment has been minimized.
This comment has been minimized.
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.
r=me with travis passing, no need to block on these comments. Just address them in a follow up. This PR will bitrot too fast otherwise
// We only want to assert that this doesn't ICE, we don't particularly care | ||
// about whether it nor it fails to compile. | ||
|
||
// error-pattern: |
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.
this is not needed anymore
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
// This file must never have a trailing newline |
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.
that comment is weird. no explanation and the file does have a trailing newline, altough not a full extra empty line at the end
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.
to be fair, the name of the file does lead us to #11493, which explicitly said that to reproduce the bug in question, we needed to have a file with no trailing newline...
| `+` can't be used to concatenate two `&str` strings | ||
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left | ||
| | ||
LL | a.to_owned() += { "b" }; |
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.
Open an issue about this wrong suggestion
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.
error: compilation successful | ||
--> $DIR/issue-11740.rs:35:1 | ||
| | ||
LL | / fn main() { //~ ERROR compilation successful |
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.
this should probably be a // compile-pass
test
--> $DIR/issue-12997-2.rs:16:1 | ||
| | ||
LL | fn bar(x: isize) { } | ||
| ^^^^^^^^^^^^^^^^^^^^ expected isize, found mutable reference |
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.
This span is weird and the message is inverted. Should probably also open an issue about it
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.
error: compilation successful | ||
--> $DIR/issue-40510-4.rs:25:1 | ||
| | ||
LL | fn main() {} //~ ERROR compilation successful |
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.
// compile-pass
error: compilation successful | ||
--> $DIR/issue-41998.rs:14:1 | ||
| | ||
LL | / fn main() { //~ ERROR compilation successful |
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.
// compile-pass
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
//~^^^^^^^^^^ ERROR cycle detected when computing layout of |
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.
needs a better span
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.
error: unnecessary `unsafe` block | ||
--> $DIR/issue-48131.rs:18:9 | ||
| | ||
LL | unsafe { /* unnecessary */ } //~ ERROR unnecessary `unsafe` |
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.
this could have a structured suggestion
| | | ||
| cannot use `+=` on type `T` | ||
| | ||
= note: `T` might need a bound for `std::ops::AddAssign` |
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.
another suggestion would be to manually expand to x = x + y
if a bound for Add is there
Thank you for the thorough checking of the output! That's part of why I wanted to move to |
@bors r=oli-obk p=10 |
📌 Commit 2332759 has been approved by |
⌛ Testing commit 2332759 with merge c2735d26cc9ae5b42d11af7d8f080fec8aa84bfb... |
💔 Test failed - status-travis |
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 |
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 |
@bors r+ p=10 |
📌 Commit f760960 has been approved by |
@bors r- |
@bors r=oli-obk p=10 |
📌 Commit f760960 has been approved by |
|
@bors r=oli-obk p=10 |
📌 Commit e0b54b5411a286f16ad89a10684dc42daa1750ec has been approved by |
⌛ Testing commit e0b54b5411a286f16ad89a10684dc42daa1750ec with merge 89c20aef0cb1552111fe1b549b193b905240ab13... |
💔 Test failed - status-travis |
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 |
1 similar comment
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 |
@bors r+ p=10 |
📌 Commit 82fd8d7 has been approved by |
Move some `compile-fail` tests to `ui` Re: #44844.
☀️ Test successful - status-appveyor, status-travis |
Re: #44844.