Stream: t-compiler/major changes

Topic: Uplift `drop-bounds` lint from clippy int… compiler-team#347


triagebot (Aug 19 2020 at 22:46, on Zulip):

A new proposal has been announced: #347. It will be announced at the next meeting to try and draw attention to it, but usually MCPs are not discussed during triage meetings. If you think this would benefit from discussion amongst the team, consider proposing a design meeting.

Michael Howell (Aug 19 2020 at 23:09, on Zulip):

triagebot said:

A new proposal has been announced: #347. It will be announced at the next meeting to try and draw attention to it, but usually MCPs are not discussed during triage meetings. If you think this would benefit from discussion amongst the team, consider proposing a design meeting.

The reason this lint is set as warn, instead of deny like in Clippy, is to avoid breaking libraries like pin-project that use the trait coherence rules to prevent client crates from implementing Drop. And it's not like T: Drop is UB or anything, just useless most of the time.

If you're using T: Drop correctly, then you know why this lint exists and know enough to disable it. But I don't want to break too much existing code. Does anybody know how this will interact with pin-project?

oli (Aug 20 2020 at 06:48, on Zulip):

we can make it deny, dependencies' lints are capped to allow anyway, so only building the crate directly will make you see the lint

matklad (Aug 20 2020 at 09:35, on Zulip):

Meta-Meta: the same "warn vs deny" issue came up with CString lint as well. Do we have lint level policy written down somewhere? It seems like it's a better process to mechanically decide by looking at the flow chart, rather than debate in each particular case.

Meta: I think rustc and clippy should use different criteria for deny. To me it seems that both lints should be deny in clippy and warn in rustc -- they are likely bugs, but not something invalid according to language per se.

nagisa (Aug 20 2020 at 13:45, on Zulip):

Note that the linked forum post _does_ link to a valid use-case for this kind of bound.

nagisa (Aug 20 2020 at 13:47, on Zulip):

and overall, I wonder, is there another way to prevent users from implementing/instantiating something for Copy types without bounding on Drop?

Manish Goregaokar (Aug 20 2020 at 14:34, on Zulip):

@matklad @oli during the clippy 1.0 rfc this was already kinda decided, clippy deny lints rougly match up to rustc warn lints (very few rustc lints are deny)

Manish Goregaokar (Aug 20 2020 at 14:35, on Zulip):

that said, not against making it deny

oli (Aug 20 2020 at 14:55, on Zulip):

I was mainly commenting on the fact that even deny is not a breaking change for dependencies

oli (Aug 20 2020 at 14:55, on Zulip):

I'm totally good with warn

Eric Huss (Aug 20 2020 at 15:23, on Zulip):

Do we have lint level policy written down somewhere?

https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-levels

(See the top of that chapter for other guidelines, style, lint vs fixed diagnostic, etc.)

Eric Huss (Aug 20 2020 at 15:23, on Zulip):

(Scroll down a bit for lint levels)

Michael Howell (Aug 26 2020 at 20:31, on Zulip):

oli said:

we can make it deny, dependencies' lints are capped to allow anyway, so only building the crate directly will make you see the lint

pin-project is using drop bounds in a macro

triagebot (Sep 17 2020 at 14:30, on Zulip):

@T-compiler: Proposal #347 has been seconded, and will be approved in 10 days if no objections are raised.

triagebot (Sep 30 2020 at 22:30, on Zulip):

This proposal has been accepted: #347.

Last update: May 07 2021 at 07:45UTC