lint: convert incoherent_fundamental_impls into hard error

[hdhoang]

Summary for affected authors: If your crate depends on one of the following crates, please upgrade to a newer version:

• gtk-rs: upgrade to at least 0.4
• rusqlite: upgrade to at least 0.14
• nalgebra: upgrade to at least 0.15, or the last patch version of 0.14
• spade: upgrade or refresh the Cargo.lock file to use version 1.7
• imageproc: upgrade to at least 0.16 (newer versions no longer use nalgebra)

implement #46205

r? @nikomatsakis

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (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.

[nikomatsakis]

This looks great, thanks!

[nikomatsakis]

@bors try

cc @rust-lang/infra -- gonna want a check-only crater run of this

[bors]

⌛ Trying commit 70a2d80 with merge ecb60b3...

[bors]

💥 Test timed out

[kennytm]

@bors try- r- retry clean

The try build is successful.

[Mark-Simulacrum]

Crater has been started in check-only mode, though we're testing a beta improved version of Crater so this could turn out to take up to 5-6 days with restarts.

[Mark-Simulacrum]

Crater results are at: http://cargobomb-reports.s3.amazonaws.com/pr-49799/index.html. 'Blacklisted' crates (spurious failures etc) can be found here. If you see any spurious failures not on the list, please make a PR against that file.

(interested observers: Crater is a tool for testing the impact of changes on the crates.io ecosystem. You can find out more at the repo if you're curious)

[hdhoang]

I'm collecting the primary failures. Meanwhile lru-disk-cache 0.2.0 with the fix is on crates.io, and that fix links to this comment with several of the primaries mentioned: #46192 (comment)

Known issues

• bow-1.0.3: https://cargobomb-reports.s3.amazonaws.com/pr-49799/ecb60b3c2f42741c46899792e9c932714c8a3147/reg/bow-1.0.3/log.txt
• glib-0.1.3: https://cargobomb-reports.s3.amazonaws.com/pr-49799/ecb60b3c2f42741c46899792e9c932714c8a3147/reg/glib-0.1.3/log.txt
• ocl-0.13.1 (not craterred itself), fixed upstream with cogciprocate/ocl@d4768a7
• rusqlite 0.10 to 0.13, upstream issue jgallagher/rusqlite#313

New

• nalgebra 0.11 to 0.14: https://cargobomb-reports.s3.amazonaws.com/pr-49799/ecb60b3c2f42741c46899792e9c932714c8a3147/reg/nalgebra-0.14.1/log.txt
• pin-api-0.2.1: https://cargobomb-reports.s3.amazonaws.com/pr-49799/ecb60b3c2f42741c46899792e9c932714c8a3147/reg/pin-api-0.2.1/log.txt This crate also wants to #[allow(incoherent_fundamental_impls)]

Spurious failure/success

• rspotify: 300s timeout
• verilog: toolchain 1 had read-only filesystem

[nikomatsakis]

@hdhoang awesome, great follow-up! I was away last week, so let me take a look.

[nikomatsakis]

@hdhoang The 'known issues' were reported in the original issue or something like that?

As for nalgebra and pin-api, cc @sebcrozet and @withoutboats, seems like these crates are relying on a bug (INCOHERENT_FUNDAMENTAL_IMPLS) that we would like to remove support for. Have you any idea how to fix those errors? (I've not looked closely)

[nikomatsakis]

OK, so, I would like to close this forwards compatibility lint, converting it into a hard error. However, I'm not psyched about breaking 131 crates. @hdhoang or @rust-lang/release, is there an easy way to understand which root crates are responsible for each failure? I'd like to see if there are a few places where we can push hard on getting a fix published in order to mitigate the impact.

[pietroalbini]

@nikomatsakis yes, there are 131 regressions. The "easiest" thing to do is to go and check all of them manually, but I see why you don't want to do that :P

Unfortunately, there is no way at the moment to automatically do that (as far as I know). I can try writing a small tool to build a dependency graph and check where the regressed crates are when I go home though.

[hdhoang]

the known issues were discovered in #46192 by its crater run, with elaboration by @arielb1 here #46192 (comment)

all but bow already have fixes, either in later versions or an unmerged branch. A large swath is from glib or rusqlite, so we need the graph and then pinging the maintainers whose dependencies pull in those.

nalgebra is the root for a bunch of regressions though, and has no fix yet.

I looked at the first error from each regression txt to determine the root primary, but that wouldn't be enough enough to build an effective dep-graph to get the eco system upgraded though.

[nikomatsakis]

I'm leaving this nominated to remind people to check their boxes.

[nikomatsakis]

(NB, I checked @aturon's box as they are absent)

[Centril]

@rfcbot reviewed

Please wake up bot... :)

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[nikomatsakis]

I'm going to approve since I think that FCP is very likely to complete without incident and we could always roll back if needed.

[nikomatsakis]

@bors r+

[bors]

📌 Commit 9982e7d has been approved by nikomatsakis

[nikomatsakis]

A 💯 ❤️ to @hdhoang for seeing this through!

[bors]

⌛ Testing commit 9982e7d with merge a614cee...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: nikomatsakis
Pushing a614cee to master...

[nnethercote]

This ended up being a big perf win for packed-simd.