Skip to content
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

Mark some edition tests as check-pass #86537

Merged

Conversation

inquisitivecrystal
Copy link
Contributor

@inquisitivecrystal inquisitivecrystal commented Jun 22, 2021

Overview

This helps with #62277. In short, there are some tests that were marked as build-pass when it was unclear whether check-pass might be more appropriate. This PR marks some of those tests as check-pass, in addition to making some incidental formatting improvements.

A brief explanation of why this is correct

These tests fall into a few buckets.

src/test/ui/dyn-keyword/dyn-2015-edition-keyword-ident-lint.rs
src/test/ui/dyn-keyword/dyn-2015-idents-in-decl-macros-unlinted.rs
src/test/ui/dyn-keyword/dyn-2015-idents-in-macros-unlinted.rs
src/test/ui/dyn-keyword/dyn-2015-no-warnings-without-lints.rs
src/test/ui/dyn-keyword/issue-56327-dyn-trait-in-macro-is-okay.rs

These test a lint for a keyword added in a new edition and the corresponding changes in keyword rules.

src/test/ui/editions/edition-feature-ok.rs
This checks that a feature related to an edition transition is valid.

src/test/ui/editions/edition-imports-virtual-2015-ambiguity.rs
This checks that imports between editions work correctly.

src/test/ui/editions/edition-keywords-2015-2015-expansion.rs
src/test/ui/editions/edition-keywords-2018-2015-expansion.rs
This checks the interaction between a change in keyword status over editions and macros.

All of the things being tested come before linking and codegen, so it is safe to use check-pass for them.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 22, 2021
@@ -3,7 +3,8 @@
// to detect or fix uses of `dyn` under a macro. Since we are testing
// this file via `rustfix`, we want the rustfix output to be
// compilable; so the macros here carefully use `dyn` "correctly."

//
// edition:2015
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need it as the default edition is 2015.

Copy link
Contributor Author

@inquisitivecrystal inquisitivecrystal Jun 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I really meant to explain that better and then forgot. We don't need it, but when the test is looking at edition specific behavior, it's clearer to have a marker in the test that shows what edition it runs on. That's presumably why that marking is already on some of the other 2015 edition-specific tests. Plus, we might change the default edition for these tests in the future. If new code is going to be written on a newer edition, it would make sense for most tests to look at the behavior of the current edition rather than the old one, so changing the default could be a good idea at some point. Given all that, I thought it made sense to add it to the tests I was changing anyway, and then I put it on the others in the same group for consistency.

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, thanks!

@JohnTitor
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 22, 2021

📌 Commit 8ad63ba has been approved by JohnTitor

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 22, 2021
…ts-check-pass, r=JohnTitor

Mark some edition tests as check-pass

## Overview
This helps with rust-lang#62277. In short, there are some tests that were marked as `build-pass` when it was unclear whether `check-pass` might be more appropriate. This PR marks some of those tests as `compile-pass`, in addition to making some incidental formatting improvements.

## A brief explanation of why this is correct
These tests fall into a few buckets.

`src/test/ui/dyn-keyword/dyn-2015-edition-keyword-ident-lint.rs`
`src/test/ui/dyn-keyword/dyn-2015-idents-in-decl-macros-unlinted.rs`
`src/test/ui/dyn-keyword/dyn-2015-idents-in-macros-unlinted.rs`
`src/test/ui/dyn-keyword/dyn-2015-no-warnings-without-lints.rs`
`src/test/ui/dyn-keyword/issue-56327-dyn-trait-in-macro-is-okay.rs`

These test a lint for a keyword added in a new edition and the corresponding changes in keyword rules.

`src/test/ui/editions/edition-feature-ok.rs`
This checks that a feature related to an edition transition is valid.

`src/test/ui/editions/edition-imports-virtual-2015-ambiguity.rs`
This checks that imports between editions work correctly.

`src/test/ui/editions/edition-keywords-2015-2015-expansion.rs`
`src/test/ui/editions/edition-keywords-2018-2015-expansion.rs`
This checks the interaction between a change in keyword status over editions and macros.

All of the things being tested come before linking and codegen, so it is safe to use `check-pass` for them.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#86393 (Add regression test for issue rust-lang#52025)
 - rust-lang#86402 (rustdoc: add optional woff2 versions of Source Serif and Source Code)
 - rust-lang#86451 (Resolve intra-doc links in summary desc)
 - rust-lang#86501 (Cleanup handling of `crate_name` for doctests)
 - rust-lang#86517 (Fix `unused_unsafe` around `await`)
 - rust-lang#86537 (Mark some edition tests as check-pass)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 00a7d5c into rust-lang:master Jun 22, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants