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

Promote missing_fragment_specifier to hard error #75516

Merged
merged 4 commits into from
Aug 18, 2020

Conversation

matklad
Copy link
Member

@matklad matklad commented Aug 14, 2020

It has been deny_by_default since 2017 (and warned for some time
before that), so it seems reasonable to promote it.

The specific technical motivation to do this now is to remove a field
from ParseSess -- it is a global state, and global state makes
extracting libraries annoying.

Closes #40107

@rust-highfive
Copy link
Collaborator

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 14, 2020
@matklad
Copy link
Member Author

matklad commented Aug 14, 2020

r? @petrochenkov

@rust-highfive rust-highfive assigned petrochenkov and unassigned varkor Aug 14, 2020
@matklad matklad force-pushed the remove-deprecation branch from 2718672 to 91ba6d4 Compare August 14, 2020 09:49
@matklad matklad added the relnotes Marks issues that should be documented in the release notes of the next release. label Aug 14, 2020
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 14, 2020
@jonas-schievink jonas-schievink added this to the 1.47 milestone Aug 14, 2020
@petrochenkov
Copy link
Contributor

The specific technical motivation to do this now is to remove a field
from ParseSess -- it is a global state, and global state makes
extracting libraries annoying.

You remove one today, tomorrow two more are added for diagnostics.
Have to run fast just to stay in the same place.

@bors r+

@bors
Copy link
Contributor

bors commented Aug 14, 2020

📌 Commit 91ba6d47ae593573f943dc5e5433e6d11682c046 has been approved by petrochenkov

@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 Aug 14, 2020
@Mark-Simulacrum
Copy link
Member

@bors rollup=never (could have perf implications though I don't really expect it)

@bors
Copy link
Contributor

bors commented Aug 15, 2020

⌛ Testing commit 91ba6d47ae593573f943dc5e5433e6d11682c046 with merge 010fa54dba0d327f682636d6702119548e5fc923...

@tmandry
Copy link
Member

tmandry commented Aug 15, 2020

@bors retry
yielding to rollup

@bors
Copy link
Contributor

bors commented Aug 15, 2020

⌛ Testing commit 91ba6d47ae593573f943dc5e5433e6d11682c046 with merge 64295ac692af2e382fe3b749c19d554ba4e2320f...

@bors
Copy link
Contributor

bors commented Aug 15, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 15, 2020
@matklad
Copy link
Member Author

matklad commented Aug 15, 2020

The failure is genuine: cargotest tests a https://github.com/dzamlo/treeify project, which hasn't been updated in four years and depends on the old clap, which has a genuine bug in the macro definition.

I think we should remove treeify from cargotest (in comparision to other projects tested, it does feel like an outlier with only a single dep (clap) and small size).

However we might want to maybe run a crater here?

@matklad
Copy link
Member Author

matklad commented Aug 15, 2020

Hm, for some reason cargotest tests fails locally for me with some git error (?!), so let's

@bors try

@bors
Copy link
Contributor

bors commented Aug 15, 2020

⌛ Trying commit a07e52b22158978c7e6282c8c0ad183fde66af70 with merge 47ca8d06b142ea091021474a428f13a79a943058...

@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 18, 2020
treeify depends on an outdated version of clap, which will fail to compile
when we promote missing_fragment_specifier future compat lint to
error (ie, old clap contains code that shouldn't have compiled in the
first place).

Additionally, this crate seem tiny relative to other crates we are
testing here, so it seems like it doesn't provide that much additional
confidence.

The same happens with tokei project, but it is an actively maintained
one, so we can just upgrade it to a version from 2018, where clap was
upgraded.
@matklad matklad force-pushed the remove-deprecation branch from e6a13f4 to eb4d6b5 Compare August 18, 2020 11:33
@matklad
Copy link
Member Author

matklad commented Aug 18, 2020

Ah, convinced myself that I did check tokei in cargo test, but that's obviously not true. I've updated version of tokei we use. I think this is fine, although a bit sad.

SO, this waits fore review now.

@petrochenkov
Copy link
Contributor

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Aug 18, 2020

📌 Commit eb4d6b5 has been approved by petrochenkov

@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 Aug 18, 2020
@bors
Copy link
Contributor

bors commented Aug 18, 2020

⌛ Testing commit eb4d6b5 with merge 30f0a07...

@bors
Copy link
Contributor

bors commented Aug 18, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: petrochenkov
Pushing 30f0a07 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 18, 2020
@bors bors merged commit 30f0a07 into rust-lang:master Aug 18, 2020
@matklad matklad deleted the remove-deprecation branch August 18, 2020 23:19
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 3, 2020
…oalbini

[beta] Revert "Promote missing_fragment_specifier to hard error rust-lang#75516"

This reverts "Promote missing_fragment_specifier to hard error rust-lang#75516" on just beta. I would like us to explore a more principled fix, perhaps along the lines `@petrochenkov` suggested in rust-lang#76605, on master when we have more time to test it but I don't want us shipping the breakage in the meantime. I don't personally feel comfortable immediately backporting anything more than a revert here.

cc `@matklad`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 7, 2020
…chenkov

[beta] Revert "Promote missing_fragment_specifier to hard error rust-lang#75516"

This reverts "Promote missing_fragment_specifier to hard error rust-lang#75516" on just beta. I would like us to explore a more principled fix, perhaps along the lines `@petrochenkov` suggested in rust-lang#76605, on master when we have more time to test it but I don't want us shipping the breakage in the meantime. I don't personally feel comfortable immediately backporting anything more than a revert here.

cc `@matklad`

This matches rust-lang#77456 for 1.47 but targets 1.48 (current beta) instead.

r? `@petrochenkov`
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Nov 13, 2020
Pkgsrc changes:
 * Remove patches now integrated upstream, many related to SunOS / Illumos.
 * The LLVM fix for powerpc is also now integrated upstream.
 * Adapt those patches where the source has moved or parts are integrated.
 * The randomness patches no longer applies, and I could not find
   where those files went...
 * Provide a separate bootstrap for NetBSD/powerpc 9.0, since apparently
   the C++ ABI is different from 8.0.  Yes, this appears to be specific to
   the NetBSD powerpc ports.

Upstream changes:

Version 1.47.0 (2020-10-08)
==========================

Language
--------
- [Closures will now warn when not used.][74869]

Compiler
--------
- [Stabilized the `-C control-flow-guard` codegen option][73893], which enables
  [Control Flow Guard][1.47.0-cfg] for Windows platforms, and is ignored on
  other platforms.
- [Upgraded to LLVM 11.][73526]
- [Added tier 3\* support for the `thumbv4t-none-eabi` target.][74419]
- [Upgrade the FreeBSD toolchain to version 11.4][75204]
- [`RUST_BACKTRACE`'s output is now more compact.][75048]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
---------
- [`CStr` now implements `Index<RangeFrom<usize>>`.][74021]
- [Traits in `std`/`core` are now implemented for arrays of any length, not just
  those of length less than 33.][74060]
- [`ops::RangeFull` and `ops::Range` now implement Default.][73197]
- [`panic::Location` now implements `Copy`, `Clone`, `Eq`, `Hash`, `Ord`,
  `PartialEq`, and `PartialOrd`.][73583]

Stabilized APIs
---------------
- [`Ident::new_raw`]
- [`Range::is_empty`]
- [`RangeInclusive::is_empty`]
- [`Result::as_deref`]
- [`Result::as_deref_mut`]
- [`Vec::leak`]
- [`pointer::offset_from`]
- [`f32::TAU`]
- [`f64::TAU`]

The following previously stable APIs have now been made const.

- [The `new` method for all `NonZero` integers.][73858]
- [The `checked_add`,`checked_sub`,`checked_mul`,`checked_neg`, `checked_shl`,
  `checked_shr`, `saturating_add`, `saturating_sub`, and `saturating_mul`
  methods for all integers.][73858]
- [The `checked_abs`, `saturating_abs`, `saturating_neg`, and `signum`  for all
  signed integers.][73858]
- [The `is_ascii_alphabetic`, `is_ascii_uppercase`, `is_ascii_lowercase`,
  `is_ascii_alphanumeric`, `is_ascii_digit`, `is_ascii_hexdigit`,
  `is_ascii_punctuation`, `is_ascii_graphic`, `is_ascii_whitespace`, and
  `is_ascii_control` methods for `char` and `u8`.][73858]

Cargo
-----
- [`build-dependencies` are now built with opt-level 0 by default.][cargo/8500]
  You can override this by setting the following in your `Cargo.toml`.
  ```toml
  [profile.release.build-override]
  opt-level = 3
  ```
- [`cargo-help` will now display man pages for commands rather just the
  `--help` text.][cargo/8456]
- [`cargo-metadata` now emits a `test` field indicating if a target has
  tests enabled.][cargo/8478]
- [`workspace.default-members` now respects `workspace.exclude`.][cargo/8485]
- [`cargo-publish` will now use an alternative registry by default if it's the
  only registry specified in `package.publish`.][cargo/8571]

Misc
----
- [Added a help button beside Rustdoc's searchbar that explains rustdoc's
  type based search.][75366]
- [Added the Ayu theme to rustdoc.][71237]

Compatibility Notes
-------------------
- [Bumped the minimum supported Emscripten version to 1.39.20.][75716]
- [Fixed a regression parsing `{} && false` in tail expressions.][74650]
- [Added changes to how proc-macros are expanded in `macro_rules!` that should
  help to preserve more span information.][73084] These changes may cause
  compiliation errors if your macro was unhygenic or didn't correctly handle
  `Delimiter::None`.
- [Moved support for the CloudABI target to tier 3.][75568]
- [`linux-gnu` targets now require minimum kernel 2.6.32 and glibc 2.11.][74163]

Internal Only
--------
- [Improved default settings for bootstrapping in `x.py`.][73964]
  You can read details about this change in the ["Changes to `x.py`
  defaults"](https://blog.rust-lang.org/inside-rust/2020/08/30/changes-to-x-py-defaults.html)
  post on the Inside Rust blog.

- [Added the `rustc-docs` component.][75560] This allows you to install
  and read the documentation for the compiler internal APIs. (Currently only
  available for `x86_64-unknown-linux-gnu`.)

[1.47.0-cfg]: https://docs.microsoft.com/en-us/windows/win32/secbp/control-flow-guard
[76980]: rust-lang/rust#76980
[75048]: rust-lang/rust#75048
[74163]: rust-lang/rust#74163
[71237]: rust-lang/rust#71237
[74869]: rust-lang/rust#74869
[73858]: rust-lang/rust#73858
[75716]: rust-lang/rust#75716
[75908]: rust-lang/rust#75908
[75516]: rust-lang/rust#75516
[75560]: rust-lang/rust#75560
[75568]: rust-lang/rust#75568
[75366]: rust-lang/rust#75366
[75204]: rust-lang/rust#75204
[74650]: rust-lang/rust#74650
[74419]: rust-lang/rust#74419
[73964]: rust-lang/rust#73964
[74021]: rust-lang/rust#74021
[74060]: rust-lang/rust#74060
[73893]: rust-lang/rust#73893
[73526]: rust-lang/rust#73526
[73583]: rust-lang/rust#73583
[73084]: rust-lang/rust#73084
[73197]: rust-lang/rust#73197
[72488]: rust-lang/rust#72488
[cargo/8456]: rust-lang/cargo#8456
[cargo/8478]: rust-lang/cargo#8478
[cargo/8485]: rust-lang/cargo#8485
[cargo/8500]: rust-lang/cargo#8500
[cargo/8571]: rust-lang/cargo#8571
[`Ident::new_raw`]:  https://doc.rust-lang.org/nightly/proc_macro/struct.Ident.html#method.new_raw
[`Range::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.Range.html#method.is_empty
[`RangeInclusive::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.RangeInclusive.html#method.is_empty
[`Result::as_deref_mut`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref_mut
[`Result::as_deref`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref
[`TypeId::of`]: https://doc.rust-lang.org/nightly/std/any/struct.TypeId.html#method.of
[`Vec::leak`]: https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.leak
[`f32::TAU`]: https://doc.rust-lang.org/nightly/std/f32/consts/constant.TAU.html
[`f64::TAU`]: https://doc.rust-lang.org/nightly/std/f64/consts/constant.TAU.html
[`pointer::offset_from`]: https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.offset_from
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 22, 2020
…ulacrum

Revert "Promote missing_fragment_specifier to hard error" rust-lang#75516

Revert of rust-lang#75516 per rust-lang#76605.

r? `@Mark-Simulacrum`

Note: I only reverted the two commits in rust-lang#75516 which made the lint a hard error. I did not revert the other two commits in the PR as they seemed fine to leave IMO (commits 84fcd0d and eb4d6b5).
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 31, 2024
This was attempted in [1] then reverted in [2] because of fallout.
Recently, this was made an edition-dependent error in [3].

Make missing fragment specifiers an unconditional error again.

[1]: rust-lang#75516
[2]: rust-lang#80210
[3]: rust-lang#128006
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 31, 2024
…unconditional, r=<try>

[crater] Make `missing_fragment_specifier` an unconditional error

This was attempted in [1] then reverted in [2] because of fallout. Recently, this was made an edition-dependent error in [3].

Experiment with turning missing fragment specifiers an unconditional error again.

More context: rust-lang#128006

[1]: rust-lang#75516
[2]: rust-lang#80210
[3]: rust-lang#128006
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue for future-incompatibility lint missing_fragment_specifier
8 participants