Fix compile_fail tag #33793
Fix compile_fail tag #33793
Conversation
| @@ -66,7 +68,7 @@ this restriction. | |||
|
|
|||
| This happens when a trait has a method like the following: | |||
|
|
|||
| ```compile_fail | |||
| ```ignore | |||
There was a problem hiding this comment.
I'm not clear on why we're ignoring these
There was a problem hiding this comment.
It shouldn't. Thanks for noticing!
|
So, this patch seems sensible, but because of the lack of description, it took me a while to figure out exactly what it was doing, and what the problem is. Could you maybe amend the commit message to describe exactly what's going on here, and squash? Thanks |
| @@ -262,17 +262,19 @@ fn runtest(test: &str, cratename: &str, cfgs: Vec<String>, libs: SearchPaths, | |||
| control.after_analysis.stop = Compilation::Stop; | |||
| } | |||
|
|
|||
| let res = panic::catch_unwind(AssertUnwindSafe(|| { | |||
There was a problem hiding this comment.
I think this reads better having the panic + closure in a variable and then later matching the variable rather than doing both at once.
There was a problem hiding this comment.
I guess it's a question of point of view? For me it's better this way but if someone else agrees with you, then I'll do it again in two steps.
There was a problem hiding this comment.
I am not sure, personally. I can see it both ways.
There was a problem hiding this comment.
Well... Thanks? 😆
|
@steveklabnik: I splitted the two commits to make the review easier. I guess it wasn't a good idea. So just in case, the first commit was to fix cases where the compilation went fine when it was expecting that it'd fail. Second case just update the cases where it now fails because of this change. |
GuillaumeGomez
force-pushed
the
compile_fail
branch
from
bbd5195 to
459da6f
Compare
|
Ok, so now commits have been squashed and commit message has been rewritten. It just remains the little syntax issue @jonathandturner reported (but I need another confirmation before removing it). |
GuillaumeGomez
force-pushed
the
compile_fail
branch
from
459da6f to
44e9ae0
Compare
|
@bors: r+ rollup |
|
📌 Commit 44e9ae0 has been approved by |
|
@bors: rollup- |
|
☔ The latest upstream changes (presumably #33833) made this pull request unmergeable. Please resolve the merge conflicts. |
GuillaumeGomez
force-pushed
the
compile_fail
branch
from
44e9ae0 to
7a5bf59
Compare
|
@bors: r=steveklabnik |
|
📌 Commit 7a5bf59 has been approved by |
|
⌛ Testing commit 7a5bf59 with merge 4572819... |
…expected to and was still considered 'ok') * Fix error explanations tests/tags
|
💔 Test failed - auto-win-gnu-32-opt-rustbuild |
GuillaumeGomez
force-pushed
the
compile_fail
branch
from
7a5bf59 to
abe9961
Compare
|
Updated. |
|
@bors: r+ rollup |
|
📌 Commit abe9961 has been approved by |
|
⌛ Testing commit abe9961 with merge cac25b7... |
|
💔 Test failed - auto-linux-64-opt-rustbuild |
|
@bors: r+ rollup |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit abe9961 has been approved by |
|
@bors: retry |
…laumeGomez Fix compile_fail tag Fixes rust-lang#33780 r? @steveklabnik
…laumeGomez Fix compile_fail tag Fixes rust-lang#33780 r? @steveklabnik
|
⌛ Testing commit abe9961 with merge e73aba1... |
|
💔 Test failed - auto-linux-64-cross-netbsd |
|
@bors: retry |
|
⌛ Testing commit abe9961 with merge 623cb7a... |
…laumeGomez Fix compile_fail tag Fixes rust-lang#33780 r? @steveklabnik
|
⛄ The build was interrupted to prioritize another pull request. |
|
☔ The latest upstream changes (presumably #33959) made this pull request unmergeable. Please resolve the merge conflicts. |
Fixes #33780
r? @steveklabnik