[Pre RFC] Cargo: report future-incompat

tools and infrastructure cargo
1

Summary

Cargo should alert developers to upstream dependencies that trigger future-incompatibility warnings. Cargo should list such dependencies even when these warnings have been suppressed (e.g. via cap-lints or #[allow(..)] attributes.)

Cargo could additionally provide feedback for tactics a maintainer of the downstream crate could use to address the problem (the details of such tactics is not specified nor mandated by this RFC).

Motivation

From rust-lang/rust#34596:

if you author a library that is widely used, but which you are not actively using at the moment, you might not notice that it will break in the future -- moreover, your users won't either, since cargo will cap lints when it builds your library as a dependency.

Today, cargo will cap lints when it builds libraries as dependencies. This behavior includes future-incompatibility lints.

As a running example, assume we have a crate unwary with an upstream crate dependency brash, and brash has code that triggers a future-incompatibility lint, such as match x { 3.4 => 5, _ => 6 }, (see rust-lang/rust#41620).

If brash is a non-path dependency of unwary, then building unwary will suppress the warning associated with brash in its diagnostic output, because the build of brash will pass --cap-lints=allow to its rustc invocation. This means that a future version of Rust is going to fail to compile the unwary project, with no warning to the developer of unwary.

Example of today's behavior (where in this case, brash is non-path dependency of unwary):

crates % cd unwary
unwary % cargo build                                                # no warning issued about problem in the `brash` dependency.
   Compiling brash v0.1.0
   Compiling unwary v0.1.0 (/tmp/unwary)
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s
unwary % cd ../brash
brash % cargo build                                                 # (but a `brash` developer will see it when they build.)
   Compiling brash v0.1.0 (/tmp/brash)
warning: floating-point types cannot be used in patterns
 --> /tmp/brash/src/lib.rs:1:37
  |
1 | pub fn f() { let x = 1.2; match x { 3.4 => 5, _ => 6 }; }
  |                                     ^^^
  |
  = note: #[warn(illegal_floating_point_literal_pattern)] on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #41620 <https://github.com/rust-lang/rust/issues/41620>
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s
brash %

Even if brash is a path-dependency of unwary (which means building unwary will issue the warning associated with brash when it builds that crate), but subsequent rebuilds of unwary will not rebuild brash, and thus will not emit the diagnostic for the future-incompatibility issue (at least not until unwary updates to a newer version of brash or of Rust itself).

Example of today's behavior (where in this case, brash is a path dependency of unwary):

crates % cd unwary
unwary % cargo build                                                # We *do* see warning on first build...
   Compiling brash v0.1.0 (/tmp/brash)
warning: floating-point types cannot be used in patterns
 --> /tmp/brash/src/lib.rs:1:37
  |
1 | pub fn f() { let x = 1.2; match x { 3.4 => 5, _ => 6 }; }
  |                                     ^^^
  |
  = note: #[warn(illegal_floating_point_literal_pattern)] on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #41620 <https://github.com/rust-lang/rust/issues/41620>
   Compiling unwary v0.1.0 (/tmp/unwary)
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s
unwary % touch src/main.rs
unwary % cargo build                                                # ... but no warning issued about still-present problem in path dependency.
   Compiling unwary v0.1.0 (/tmp/unwary)
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s
unwary %

Therefore, if the developer does not do something to address the warning the first time they see it, it is likely to go unaddressed until the developer upgrades to a version of Rust that actually fails to compile brash. This is a frustrating developer experience.

Cargo passes --cap-lints=allow on upstream dependencies for good reason, as discussed in Rust RFC 1193 and the comment thread from rust-lang/rust#59658. For cases like future-incompatibility lints, which are more severe warnings for the long-term viability of a crate, we need to provide some feedback to the unwary maintainer.

But this feedback should not be just the raw diagnostic output for brash! The developer of a crate like unwary typically cannot do anything in the short term about warnings emitted by upstream crates, and therefore the diagnostics associated with building upstream brash are usually just noise from the viewpoint of unwary's maintainer.

Therefore, we want to continue passing --cap-lints=allow for upstream dependencies. But we also want rustc to tell cargo (via some channel) about when future-incompatibility lints are triggered, and we want cargo to provide a succinct report of the triggers.

Furthermore, we want the feedback to provide guidance as to how the unwary maintainer can address the issue. Here are some potential forms this additional guidance could take.

  • If cargo is not running in "offline mode", it can take the future-incompatibilty signaling as an opportunity to query crates.io to find out if a newer version of the upstream crate is available, and if so, suggest to the user they might upgrade to it. If such an upgrade could be done via cargo update, then the output could obviously suggest that as well. (This is just a heuristic measure, as it would not attempt to check ahead of time if the newer version actually resolves the problem in question.)

  • Cargo could suggest to the unwary maintainer that they file a bug (or search for previously-filed bug) in the source repository for the upstream crate that is issuing the future-incompatibility warning. (That is, the brash author might not be aware of the issue; for example, if they last updated their crate before the lint in question was deployed on the Rust compiler.)

  • rustc itself could embed, for each future-incompatibility lint, how soon the Rust developers will turn the lint to a hard error. This would give the unwary maintainer an idea of how much time they have before they will be forced to address the issue (by posting a PR upstream, or switching to a fork of brash, et cetera).

This RFC suggests the provided feedback take the form of a summary at the end of the build of unwary, as illustrated in the explanation below.

Guide-level explanation

After cargo finishes compiling a crate and its upstream dependencies, it may include a final warning about future incompatibilties.

A future incompatibility is a pattern of code that is scheduled to be removed from the language in some future release. Such code patterns are usually instances of constructs that exhibit undefined behavior (i.e. they are unsound), but are in widespread use and thus need a grace period before they are removed.

If any crate or any of its upstream dependencies has code that triggers a future incompatibility warning, but the overall compilation is otherwise without error, then cargo will report all instances of crates with future incompatibilities at the end of the compilation. When possible, this report includes the future date or release version where we expect Rust to stop compiling the code in question.

Example:

crates % cd unwary
unwary % cargo build
   Compiling brash v0.1.0
   Compiling bold v0.1.0
   Compiling rash v0.1.0
   Compiling unwary v0.1.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

    warning: the crates brash, bold, and rash contain code that will be rejected by a future version of Rust.
    note: the crate rash will stop compiling in Rust 1.50 (scheduled for February 2021).
unwary %

Rebuilding unwary continues to emit the report even if the upstream dependencies are not rebuilt.

Example:

unwary % touch src/main.rs
unwary % cargo build
   Compiling unwary v0.1.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

    warning: the crates brash, bold, and rash contain code that will be rejected by a future version of Rust.
    note: the crate rash will stop compiling in Rust 1.50 (scheduled for February 2021).
unwary %

To keep the user experience consistent, we should probably emit the same warning at the end even when the root crate is the sole trigger of incompatibility lints.

crates % cd brash
brash % cargo build
   Compiling brash v0.1.0 (/tmp/brash)
warning: floating-point types cannot be used in patterns
 --> /tmp/brash/src/lib.rs:1:37
  |
1 | pub fn f() { let x = 1.2; match x { 3.4 => 5, _ => 6 }; }
  |                                     ^^^
  |
  = note: #[warn(illegal_floating_point_literal_pattern)] on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #41620 <https://github.com/rust-lang/rust/issues/41620>
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

    warning: the crate brash contains code that will be rejected by a future version of Rust.
brash % cargo build
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s

    warning: the crate brash contains code that will be rejected by a future version of Rust.
brash %

And as you might expect, if there are no future-incompatibilty warnings issused, then the output of cargo is unchanged from today. Example:

crates % cd unwary
unwary % cargo build
   Compiling brash v0.2.0
   Compiling bold2 v0.1.0
   Compiling unwary v0.2.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

Here, the unwary (sic) crate has updated its version of brash, switched to bold2 (a fork of bold), and replaced its internal usage of rash with some local code, thus completely eliminating all current future-incompatibility lint triggers.

Reference-level explanation

As noted above, we want to continue to suppress normal lint checks for upstream dependencies. Therefore, Cargo will continue to pass --cap-lints=allow for non-path upstream depedencies.

However, the Rust compiler's behavior will change slightly. Even when --cap-lints=allow is turned on, we need Cargo to know when a future-incompatibilty lint is triggered.

The division of responsbiilties between Cargo and the Rust compiler may be a little subtle:

The responsibilities of the Rust compiler (rustc):

  • rustc must differentiate future-incompatibility lints (c.f. PR #59658: "Minimum Lint Levels") from other lints that are expected to remain as mere warnings forever.

  • rustc will need to have some new mode of operation, which this RFC will call the where it will check for instances of future-incompatibility lints, regardless of whether --cap-lints=allow is also set. This RFC calls this the future-incompatibility checking mode.

    • In the future-incompatibility checking mode of invocation, rustc will also need to check for such lints regardless of whether the code appears in the scope of an #[allow(..)] attribute for the lint.

    • In the future-incompatibility checking mode of invocation, emission of the diagnostics themselves may still be silenced as specified by --cap-lints=allow or #[allow(..)] attributes.

    • That is, those flags and annotations should be interpreted by rustc as silencing the diagnostic report, but not as silencing the feedback about there being some instance of the triggering somewhere in the crate's source code.

    • The future-incompatibility checking mode is meant as a way to address the bulk of issue rust-lang/rust#34596.

The responsibilities of Cargo:

  • Cargo is responsible for invoking rustc in a way that enables the future-incompatibility checking mode. This mode of invocation occurs regardless of whether --cap-lints=allow is also being passed when the crate is compiled.

  • Cargo is responsible for capturing any output from the future-incompatibility checking mode and summarizing it at the end of the whole build.

  • Cargo is responsible for storing a record of any future-incompatibility for a crate somewhere in the build/ directory, so that it can emit the same report without having to rebuild the crate.

  • Cargo is responsible for suggesting ways to address the problem to the user. The specific tactics for constructing such suggestions are not mandated by this RFC, but some ideas are presented as Future possibilities.

Implementation strategy: Leverage JSON error-format

The cleanest way to implement the above division of responsibilities without perturbing non-cargo uses of rustc is probably to make two changes:

  • Cargo should always use --error-format=json for its rustc invocations, and

  • rustc should treat --error-format=json as signal that it should emit a future-incompatibility summary report for the crate.

It is relatively easy to extend the JSON output of rustc to include a new record of any future-incompatibility lints that were triggered when compiling a given crate.

  • However, this RFC does not dictate this choice of implementation strategy. (Other options include using some environment variable to opt-in to a change to rustc's output, or having rustc emit future-incompatibility metadata to the filesystem.)

Some (but not all) future-incompatibility lints will have a concrete schedule established for when they are meant to become hard errors. (This RFC does not specify the details about how such schedules are established or what constraints they will have to meet; it just posits that they will be established by some means.) The metadata for every future-incompatibility lint should include the anticipated version of Rust, if known, where it will become a hard error.

  • This adds motivation for using JSON formatted diagnostics: JSON records are more readily extensible to support this sort of feedback in a robust fashion.

Since cargo is expected to continue to emit the report even when the upstream dependencies are not rebuilt, Cargo will store the future-incompatibility status for each crate somewhere in the build/ directory on the file-system. (This is analogous to how incremental compilation caches diagnostic output, so that future runs will still emit the same report even when we do not recompile the same input.)

Drawbacks

The change as described requires revisions to both rustc and cargo, which can be tricky to coordinate.

This RFC suggests an approach where the changes are somewhat loosely coupled: Use of --error-format=json will enable the future-compatibility checking mode. This avoids the need to add a new stable command line flag to rustc; but it also may be a confusing change in compiler semantics for any non-cargo client of rustc that is using --error-format=json.

Emitting a warning on all following cargo invocations until the problem is resolved may be overkill. In particular, it may not be reasonable for someone to resolve this problem in the short term. (Perhaps we could restrict it so that it only gets re-emitted once each day or something; but of course then the diagnostic would have to state very clearly that it is doing that strange magic.)

Rationale and alternatives

No change needed?

Some claim that "our current approach" has been working, and therefore no change is needed here. However, my counterargument is that the only reason we haven't had to resolve this before is that compiler and language teams have been relatively conservative in changing existing future-incompatibility lints into hard errors.

Is there something simpler?

The immediate simple approaches to resolving this problem would have serious drawbacks.

Can be do this in Cargo alone?

With regards to implementation, we could avoid attempt making changes to rustc itself, and isolate the implementation to cargo alone. The main way I can imagine doing this is to stop passing --cap-lints=allow, and then having Cargo capture all diagnostic output from the compiler and post-processing it to determine which lints are future-incompatible warnings. However, ths has a number of problems:

  • It is fragile, since it relies on Cargo post-processing the compiler diagnostic output.

  • It is inefficient, since the compiler will now always run all the lints checks for all dependencies of a crate but we only care about a small subset of the lints.

  • It is insufficient, because it only handles instances of --cap-lints; it would fail to catch instances where an upstream dependency is using an #[allow(..)] attribute in the source to sidestep warnings.

    • If we addressed that insufficiency by unconditionally changing rustc to always emit feedback about future-incompatibilities regardless of --cap-lints=allow or #[allow(..)], then that would probably upset people who expect those flags/annotations to keep the diagnostic output quiet. (In other words, that would be an instance of "the compiler is not listening to me!")

Can we do this in Rust alone?

PR rust-lang/rust#59658 "Minimum Lint Levels" implemented a solution in the compiler alone, by tagging the future-incompatibility lints as special cases that would not be silenced by --cap-lints nor #[allow(..)]. The discussion on that PR described a number of problems with this; in essence, people were concerned about getting spammed by lints that the downstream developer couldn't actually do anything about.

The discussion on that PR concluded by saying that it could possibly be reworked to reduce the amount of spam by reporting a single instance of a lint for each dependency (rather than having a separate diagnostic for each expression that triggered the lint within that dependency).

  • The latter would indeed be an improvement on PR #rust-lang/rust#59658, but it would not be an ideal user-experience. The change suggested by this RFC deliberately treats occurrences of future-incompatibility lints as separate from normal diagnostics: serious events worthy of being treated specially by Cargo, to the extent that it would do an online query to see if a newer version of the given crate exists. We want to make the process of fixing these issues as easy as we can for the developer, and doing that requires help from Cargo.

Prior art

None I know of, but I'm happy to be educated.

(Has Python done anything here with the migration from Python 2 to Python 3? I briefly did some web searches but failed to find much of use.)

Unresolved questions

There are future-incompatibility warnings emitted by cargo itself, such as discussed on rust-lang/cargo#6313 (comment). I imagine it shouldn't require much to incorporate support for them, but it is something I haven't explicitly addressed nor investigated.

Implementation questions

  • Is using --error-format=json as the way to switch rustc into the future-incompatibility checking mode reasonable?

  • Is turning on --error-format=json by default everywhere in cargo reasonable?

Future possibilities

The main point I envisage for follow-up work to this RFC is the form of feedback that Cargo gives regarding the issues.

Cargo is responsible for suggesting to the users ways to address an instance of future-incompatbility.

Following are some ideas for suggestions.

Query for newer/alternate versions of the crate

When crates trigger future-incompatibility warnings, Cargo could look for newer versions of the dependency on crates.io.

Example:

crates % cd unwary
unwary % cargo build
   Compiling brash v0.1.0
   Compiling bold v0.1.0
   Compiling rash v0.1.0
   Compiling unwary v0.1.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

    warning: the crates brash, bold, and rash contain code that will be rejected by a future version of Rust.
    note: the crate rash will stop compiling in Rust 1.50 (scheduled for February 2021).
    note: newer versions of bold and rash are available; upgrading to them via `cargo update` may resolve their problems.
unwary %

This suggestion as written only covers upgrading to newer versions. But with more help from crates.io itself, we could go even further here: We could suggest potential forks of the upstream crate that one might switch to using. This could be useful in dealing with abandonware.

Suggest a bug report

If no newer version of the triggering crate is available, Cargo could print a template for a bug report the user could file with the upstream crate.

Example:

crates % cd unwary
unwary % cargo build
   Compiling brash v0.1.0
   Compiling unwary v0.1.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

    warning: the crate brash contains code that will be rejected by a future version of Rust.
    note: the following gist contains a bug report you might consider filing with the maintainer of brash.
          https://gist.github.com/pnkfelix/ae03d3ea95160fb71a797b15e05f8d49
unwary %

For ease of reference, here is the text located at the gist url above (brash: future incompat issue · GitHub):

This crate currently triggers a future incompatibility warning with Rust.

In src/lib.rs:1:37, there is the following code:

pub fn f() { let x = 1.2; match x { 3.4 => 5, _ => 6 }; }

This causes rustc to issue the following diagnostic, from https://github.com/rust-lang/rust/issues/41620

warning: floating-point types cannot be used in patterns
 --> /tmp/brash/src/lib.rs:1:37
  |
1 | pub fn f() { let x = 1.2; match x { 3.4 => 5, _ => 6 }; }
  |                                     ^^^
  |
  = note: #[warn(illegal_floating_point_literal_pattern)] on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #41620 <https://github.com/rust-lang/rust/issues/41620>

Since this construct is going to become a hard error in the future, we should eliminate occurrences of it.

Further refinement of this idea: If we did start suggesting bug report templates, then Cargo might also be able to search for occurrences of the template on that crate's repostory, and advise the user to inspect that bug report to see its current status, rather than file a new bug with the upstream crate, which might be annoying for those maintainers.

13 Likes
2

On the one hand, I think it would be very useful to surface future incompatibility warnings from dependent crates when they get built. On the other hand, I feel like having them emitted for every compile will be very annoying. Depending on the responsiveness of the upstream crate, the warnings might not be immediately actionable. I wonder if there is some compromise solution here; for example, crates typically get rebuilt every 6 weeks anyway because you pick up a new compiler, so maybe just restrict it to when the crate actually gets compiled? Or silence the warning for a week after it's been generated, potentially after a manual acknowledgement?

1 Like
3

yep I had thoughts along these lines; I tried to include it in the Drawbacks section but maybe it wasn't clearly written

4

As you note yourself later, the developer of unwary can do something short-term about brash's problems: they can file an issue or a PR against the crate and contribute towards the solution.

As unwary's maintainer I'd want to be informed of why (the diagnostic says so) and where (the diagnostic's span says so) the problem exists, not just that it exists. It would be good if this RFC proscribed a way for the user to tell cargo to display those warnings ("to see what the problems were, use --my-flag") but I see that you've included the gist idea, which is good.

I would note that minimum-lint-levels and being treated specially by Cargo are not in opposition. They could be combined by both giving inline de-duplicated warnings when compiling the crate as well as a summary at the end (with possible actionable suggestions). Moreover, using minimum lint levels has the advantage of working without cargo which some larger companies do not use.

I think a bit of annoyance is actually by design. The annoyance will move people to action by filing issues or PRs to upstream. I think that's justified as C-future-compat issues can be serious problems (e.g. when they are soundness holes.)

I should end by saying that the most important thing is doing something sooner rather than later.

1 Like
5

I, for one, would argue that rust should only turn future incompatibility notices into hard errors in new editions, meaning brash would never break under the unwary's maintainer feet in the first place.

2 Likes
6

It's a non-starter to leave e.g. soundness holes open on certain editions and I don't see why we'd use editions, a mechanism that exists to break things for which we do guarantee stability, for things which we do not (like bugs).

1 Like
7

The example given is not a soundness hole, is it?

8

You made a general point about C-future-compat issues, not this specific one. The running example of the C-future-compat lint is a case of a bug (as decided per RFC) rather than a soundness hole. If you want to discuss that bug further, please do so on the relevant tracking issues. Editions are however not for fixing mistakes in the implementation.

1 Like
9

I'm somewhat weary of the idea of implementing this as cargo magic, rather than enabling rustc with a feature like -S, --summarize OPT Summarize the respective lint, and having cargo enable that?

One of my biggest frustrations, is with the lack of configurability around lints #5034, regarding dependencies, and transitive dependencies... a separate issue but should cargo implement this on it's lonesome specifically for the future-incompatible lint. That would make rust capable of applying lints that it wants on transitive dependencies while ignoring in some cases the

.cargo/config rustflags = "--forbid ..."

having this machinery without the ability to configure it for the user strikes me unfortunate, and I'd much rather see better lint configurability and a summary option to rustc that people could configure, than baking these specific behaviors into cargo.

That is to say, I would vastly prefer it if for this behavior cargo limited itself to setting appropriate defaults and not hard coding specific behaviors on the future-incompatability lint which cannot be configured for other lints.

10

I don't understand the proposal here. Are you suggesting that this unspecified --summarize option would somehow allow the machinery to live entirely within rustc?

Part of the goal of the RFC as written is to make the feedback regarding future-incompatibilities succinct; that is the main reason why I am proposing putting machinery into Cargo: So that there can be a few lines of output (perhaps just one) at the end of the compilation of all of the crates, which spans many invocations of rustc.

That is, there are customers of Rust, such as Servo, that bristle at the idea of being spammed with more diagnostic output that they will end up just ignoring.

But for Cargo to know that it needs to emit the succinct output at the end of building the crate graph, it needs to know that future-incompatibility lints were triggered. So it needs feedback from rustc as it runs, as discussed in the RFC text.

Is the problem with the fact that future-incompatibility lints are being given special treatment, and that this functionality specified here (which spans both cargo and rustc by necessity) should be generalized so that the user could specify a broader collection of lints that they want to receive this treatment?

I'd write down more questions prompted by your post, but I suspect we would both be better served if you spelled out a bit more what --summarize even is, and what solution you envisage to this problem with the use of --summarize (perhaps combined with --forbid -- though I suspect that would be too severe an approach and thus not a solution for the problem I am describing here)

11

It is arguably a long-standing compiler bug.

But I certainly don't want discussion of this RFC to get bogged down in debate about whether certain lints should not be included in this treatment.

My claim is that there exist at least some future-incompatibility lints that today are not being addressed by upstream crates, and their existence (or more importantly, their usage by downstream clients) impedes progress on making those into hard errors.

Based on this dialogue, I will probably revise the RFC text to:

  1. Use a different example future-incompat lint, that is more obviously a compiler bug
  2. Add some text stating that we will make case-by-case decisions about whether to promote each existing future-incompat lint into the set that falls under this mechanism. (That is, there may be exceptions that we would prefer to e.g. use an edition break for introducing the hard error.)
1 Like
12

This is an interesting point.

I think the official answer today for people who want that kind of feedback is that they are supposed to use cargo --verbose or perhaps even cargo -vv. (I'm pretty sure at least one of those causes the --cap-lints to switch from allow to warn, which then causes the underlying diagnostics to be visible.)

But I will admit up front that this is not really a palatable solution, because that will produce a lot of output, including output for all of the non-future-incompat diagnostics in upstream creates. That is not a useful text to wade through.

Maybe some way to tell a cargo invocation that a specified subset of the lints are not to be capped would be a reasonable way to get the feedback you are asking for here.

13

Not entirely within rustc, cargo would have to sniff the option and provide some behavior, The behavior of --summarize and cargo I imagine would have to be something like like:

  • --summarize would not affect the lint level, like the other lint options, it would continue allow/warning/forbidding as specified,

  • It would run the lint collect data about whether it was hit, and it would emit this at the end of compilation one time. Omitting context, or providing context seperately in a way which is easily uniquely collapsible by external tools. I.e. you could grep '^summary:' | sort | uniq for each message exercised there would be 1 line of output.

Finally cargo performs the above unique'ing and adds crate names, alternately cargo pipes the summary information to some summarizer which makes the final summary

Yes that future-incompatibility lints are being given special treatment. Programs like cargo-geiger already build summaries for lints like unsafe_code. This would add a very similar behavior for future-incompatability to cargo and so It's at least worthwhile considering how it might fall fit together ideally.

If cargo ever accepts an implementation of something like #5034 It would be best if this summary behavior could be configured by the same mechanism that allow, deny , forbid, and all this is currently bottoming out in the compiler ensures that this mechanism needs to take this option into account.

That said it is a bit different than allow, deny, forbid in that it is more like an option than an enum. Does that clarify the matter?

14

I think that a single line would be quite easily missed. It should be more noticeable and yes, more annoying than that.

With all due respect to Servo, they are far from everyone.

That seems out of scope for this RFC. Any decision to turn a C-future-compatibility lint (a deviation in rustc from the spec) into an edition breaking change rather than eventually turning it into a hard error will need the full language team's approval (and speaking personally, it's unlikely that I would approve of that). The default is always to turn deviations from the spec into eventual errors.

15

I totally agree that we need a bit of annoyance. I just worry about the amount of it. Please also think of brash's maintainer, who has a full-time non-Rust job, 6 month old and 3 year old kids, is maintaining brash in their limited free time and is now beset by people who have been annoyed into spamming them on an issue for this. (That is, the amount of annoyance generated in the downstream will somewhat converge upstream -- and getting that balance right seems pretty tricky.)

1 Like
16

That seems warranted. However, I put it to you that a single de-duplicated warning from a crate is the right balance. It's not that much and @pnkfelix is suggesting something even milder than that.

One would hope that people look at the issue tracker and see that there's already an issue / PR open for the problem. Ideally, the compiler is also providing an actionable suggestion that helps in fixing the problem quickly. It's also worth noting that C-future-compatibility issues are rare. They only happen if crater finds that the number of regressions are too many and that we cannot fix the bug outright.

1 Like
17

I apologize for being overly negative, but from my perspective you’re basically saying: “It’s worth noting that warning users about breaking changes is rare. Most of the time we make breaking changes with no warning.”

Because of course, not all code is on crates.io.

I still think this RFC is probably a good idea, since some things really are worth breaking changes – namely, fixes for soundness holes – and it’s good to make sure users are forewarned about them. But I’d hate if the warning system helped normalize the idea of “future incompatibility” among the community any more than it already is…

2 Likes
18

The implication I am making here is that it is unlikely that brash's maintainer will be beset by annoyed people because:

  1. C-future-compatibility lints are rare:

    a) If we break something with no warning outright it is because the regressions were so few that we managed to opened PRs for those crates and they have been merged.

    b) Most bugs occur in niche situations, otherwise they tend to be reported quickly. NLL-migration-sized regressions are one-offs. (Also worth noting that NLL migration had unconditional warnings and thank god for that...)

  2. If the crate is important then there's already likely an issue or PR open.

These days, crater indexes a whole lot more than just crates.io. A lot of the time these are abandon-ware.

19

Thank you for providing further details of your proposal.

I will have to reflect on it. My gut feeling is that we should keep the amount of machinery added to rustc minimal here, simply because a lot of the functionality I would like to see is a better match for living in cargo, in my opinion.

Having said that, I can appreciate that we should keep in minds the needs of non-cargo clients using rustc, and it would be best if whatever machinery we do add to rustc can be readily adapted to those other clients as well.

I suspect that in the end, we will want a couple different solutions in tandem here. As @Centril has pointed out (or at least according to my interpretation of some of their feedback), other machinery like minimum-lint-levels are not opposition to changes like the one proposed here. We may well end up adopting some solution like that, or something more general such as what has been proposed in cargo#5034.

20

My text was explicitly treating Servo as a representative example of a non-trivial set of customers.

I don't know what you are trying to establish with your response to my text. It makes it sound like Servo's experience is an exceptional case (though you have chosen language that is vague enough that you could back away from such an interpretation).

I am standing by my assertion: I do not think Servo is an exceptional case here.

Spamming users with diagnostic output is not a good user experience, especially diagnostic output that they cannot resolve themselves in the short term. In my own personal experience, a slew of warnings tends to obscure the important messages.

1 Like