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

Transition future compat lints to {ERROR, DENY} - Take 2 #65785

Merged
merged 8 commits into from
Nov 8, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Oct 25, 2019

Follow up to #63247 implementing #63247 (comment).

r? @varkor
cc @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 25, 2019
@Centril Centril changed the title [WIP] Transition future compat lints to {ERROR, DENY} - Take 2 Transition future compat lints to {ERROR, DENY} - Take 2 Oct 25, 2019
@rust-highfive

This comment has been minimized.

@petrochenkov petrochenkov self-assigned this Oct 25, 2019
@rust-highfive

This comment has been minimized.

@Centril
Copy link
Contributor Author

Centril commented Oct 25, 2019

The PR builder is finally happy. 🎉

@petrochenkov petrochenkov removed their assignment Oct 25, 2019
@petrochenkov
Copy link
Contributor

Also, legacy_ctor_visibility had a large number of regressions in the crater run in #63247, but most of them from unstable syntex_syntax (expected function, found struct ast::Name), so it should be ok.

@Centril Centril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2019
@Centril Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 27, 2019
@Centril
Copy link
Contributor Author

Centril commented Oct 27, 2019

@petrochenkov @varkor This should be ready again for review now.

@bors

This comment has been minimized.

@Centril

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@JohnCSimon

This comment has been minimized.

@bors

This comment has been minimized.

@bors

This comment has been minimized.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 6, 2019

I'm all for this, but the precedent also had fix-PRs open for the crates that will break and did a reverse dependency analysis.

I know that doing so is a lot of work, but we do have our stability story. Picking an arbitrary threshold (like 5) of broken crates where it's ok to go to an error, seems a bit odd to me. Then again, we have to do this PR at some point and the forward compat lints have been triggering for a while.

I'm unsure how to proceed, thus my question about talking this through in a team meeting or just giving everyone a chance to sign off or comment.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 8, 2019

Seems like all the crates affected are abandoned (which would explain why they haven't been fixed).

@bors r+

@bors
Copy link
Contributor

bors commented Nov 8, 2019

📌 Commit 574d2b8 has been approved by oli-obk

@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 Nov 8, 2019
Centril added a commit to Centril/rust that referenced this pull request Nov 8, 2019
Transition future compat lints to {ERROR, DENY} - Take 2

Follow up to rust-lang#63247 implementing rust-lang#63247 (comment).

- `legacy_ctor_visibility` (ERROR) -- closes rust-lang#39207
- `legacy_directory_ownership` (ERROR) -- closes rust-lang#37872
- `safe_extern_static` (ERROR) -- closes rust-lang#36247
- `parenthesized_params_in_types_and_modules` (ERROR) -- closes rust-lang#42238
- `duplicate_macro_exports` (ERROR)
- `nested_impl_trait` (ERROR) -- closes rust-lang#59014
- `ill_formed_attribute_input` (DENY) -- transitions rust-lang#57571
- `patterns_in_fns_without_body` (DENY) -- transitions rust-lang#35203

r? @varkor
cc @petrochenkov
@bors
Copy link
Contributor

bors commented Nov 8, 2019

⌛ Testing commit 574d2b8 with merge 230ebbbe9d0ea2faf5fe586b8f3f049b6f571c4b...

Centril added a commit to Centril/rust that referenced this pull request Nov 8, 2019
Transition future compat lints to {ERROR, DENY} - Take 2

Follow up to rust-lang#63247 implementing rust-lang#63247 (comment).

- `legacy_ctor_visibility` (ERROR) -- closes rust-lang#39207
- `legacy_directory_ownership` (ERROR) -- closes rust-lang#37872
- `safe_extern_static` (ERROR) -- closes rust-lang#36247
- `parenthesized_params_in_types_and_modules` (ERROR) -- closes rust-lang#42238
- `duplicate_macro_exports` (ERROR)
- `nested_impl_trait` (ERROR) -- closes rust-lang#59014
- `ill_formed_attribute_input` (DENY) -- transitions rust-lang#57571
- `patterns_in_fns_without_body` (DENY) -- transitions rust-lang#35203

r? @varkor
cc @petrochenkov
@Centril
Copy link
Contributor Author

Centril commented Nov 8, 2019

@bors retry rolled up.

bors added a commit that referenced this pull request Nov 8, 2019
Rollup of 5 pull requests

Successful merges:

 - #65785 (Transition future compat lints to {ERROR, DENY} - Take 2)
 - #66007 (Remove "here" from "expected one of X here")
 - #66043 (rename Memory::get methods to get_raw to indicate their unchecked nature)
 - #66154 (miri: Rename to_{u,i}size to to_machine_{u,i}size)
 - #66188 (`MethodSig` -> `FnSig` & Use it in `ItemKind::Fn`)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Nov 8, 2019

⌛ Testing commit 574d2b8 with merge 1465351c3dbabfa79ff87afa4e90c34d77be68ae...

@bors bors merged commit 574d2b8 into rust-lang:master Nov 8, 2019
@rust-highfive
Copy link
Collaborator

Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

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 @TimNN. (Feature Requests)

@Centril Centril deleted the compat-to-error-2 branch November 8, 2019 19:12
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 29, 2019
resolve: Minor cleanup of duplicate macro reexports

Enabled by rust-lang#65785 which changed `duplicate_macro_exports` from a lint to a hard error.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Dec 30, 2019
resolve: Minor cleanup of duplicate macro reexports

Enabled by rust-lang#65785 which changed `duplicate_macro_exports` from a lint to a hard error.
kulp added a commit to kulp/tyrga that referenced this pull request Apr 11, 2020
It was not initially clear to me why, after changing
`.filter(...).next()` to `.find(...)`, the borrow checker as of Rust
1.41.1 required me to inline `all.lines()` at this point :

    error[E0596]: cannot borrow `lines` as mutable, as it is not declared as mutable
      --> tyrga-bin/../build.rs:53:33
       |
    52 |                     let lines = all.lines();
       |                         ----- help: consider changing this to be mutable: `mut lines`
    53 |                     let wrote = lines
       |                                 ^^^^^ cannot borrow as mutable

    error: aborting due to previous error

    For more information about this error, try `rustc --explain E0596`.

but it [appears][0] that the new error is [expected][1], and that if I
had been using `.find()` previously instead of `.filter(...).next()`, I
would have seen a non-fatal "compat" lint under previous compiler
versions, leading me in the same direction.

[0]: rust-lang/rust#68729
[1]: rust-lang/rust#65785
TheNeikos added a commit to TheNeikos/interoptopus that referenced this pull request Nov 25, 2024
The panics are removed as rust-lang/rust#65785
has made them an error and the compiler will complain anyway for ill
formed attributes.

The use _could_ allow them, technically. But this doesn't really exist:

https://github.com/search?q=allow%28ill_formed_attribute_input%29&type=code

Shows 4 occurences at time of commit. I would also think that users that
enable this, should know what they are doing ;)

Signed-off-by: Marcel Müller <[email protected]>
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
7 participants