Stream: t-compiler/major changes

Topic: Add a `NOOP_METHOD_CALL` lint for methods… compiler-team#375


triagebot (Oct 21 2020 at 23:43, on Zulip):

A new proposal has been announced: Add a NOOP_METHOD_CALL lint for methods which should never be directly called #375. 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.

Joshua Nelson (Oct 21 2020 at 23:44, on Zulip):

is there an existing clippy lint for this?

Aaron Hill (Oct 21 2020 at 23:53, on Zulip):

let me check

Aaron Hill (Oct 21 2020 at 23:53, on Zulip):

if so, I think we should uplift it

Aaron Hill (Oct 21 2020 at 23:54, on Zulip):

there's a clone_double_ref for clone specifically

Aaron Hill (Oct 21 2020 at 23:55, on Zulip):

but there's no generic lint that covers other noop methods on &T

Aaron Hill (Oct 21 2020 at 23:55, on Zulip):

so, I think we could remove the clippy one if we add the rustc one

Joshua Nelson (Oct 21 2020 at 23:56, on Zulip):

the general case sounds hard to do - would there be a fixed list of no-ops? Or would the lint look for functions implemented as the identity function?

Aaron Hill (Oct 21 2020 at 23:56, on Zulip):

I don't think we'd want to do any autodetection

Aaron Hill (Oct 21 2020 at 23:56, on Zulip):

I proposed a #[noop] attribute in the MCP

Aaron Hill (Oct 21 2020 at 23:57, on Zulip):

though that doens't cover all of the cases in libstd

Aaron Hill (Oct 21 2020 at 23:57, on Zulip):

all of the methods I listed in the MCP are essentially the same (and any known calls to them can be removed), so I think it makes sense to have a single lint for all of them

scottmcm (Oct 22 2020 at 00:58, on Zulip):

conceptually: :+1:

scottmcm (Oct 22 2020 at 00:58, on Zulip):

:bike::derelict_house:: noop doesn't really seem right for this to me. Clone::clone(&true) is silly, but not exactly a no-op.

scottmcm (Oct 22 2020 at 00:59, on Zulip):

Trouble for the attribute: there's no specific <&T as ToOwned>::to_owned today. That comes from the blanket ToOwned for Clone types.

Aaron Hill (Oct 22 2020 at 03:43, on Zulip):

Yeah - I discussed a couple of approaches to solving that in the mcp

Aaron Hill (Oct 22 2020 at 03:45, on Zulip):

If we start out by just linting std types, we don't have to commit ourselves to any particular way of marking the method

Aaron Hill (Oct 22 2020 at 03:45, on Zulip):

Also, I'm find with calling the attr something other than noop - it was the first thing that came to mind

Aaron Hill (Oct 28 2020 at 17:14, on Zulip):

There hasn't been any discussion of this for a week. Is anyone opposed to the overall concept of the lint (leaving aside the exact name and attributefor the moment)?

Joshua Nelson (Oct 28 2020 at 17:15, on Zulip):

SGTM, but I'm still a little unclear exactly how it will be implemented. I guess that doesn't need to block the MCP though.

Aaron Hill (Oct 29 2020 at 16:05, on Zulip):

Since this is a new lint, does this require a T-Lang FCP?

nikomatsakis (Oct 29 2020 at 22:10, on Zulip):

@Aaron Hill usually we FCP the PR

nikomatsakis (Oct 29 2020 at 22:10, on Zulip):

but you could FCP before-hand too

Aaron Hill (Oct 29 2020 at 22:13, on Zulip):

Does that happen before or after it's seconded?

nikomatsakis (Oct 29 2020 at 22:20, on Zulip):

so I'm not sure if my script will pick up MCP changes for t-lang :)

nikomatsakis (Oct 29 2020 at 22:20, on Zulip):

you could file an issue and nominate it for now

nikomatsakis (Oct 29 2020 at 22:20, on Zulip):

we should tweak these procedures

nikomatsakis (Oct 29 2020 at 22:21, on Zulip):

(been on my mind to try and harmonize things a bit...)

nikomatsakis (Oct 29 2020 at 22:21, on Zulip):

alternatively file a lang-team proposal :)

nikomatsakis (Oct 29 2020 at 22:21, on Zulip):

any which way to get it on the triage meeting agenda

nikomatsakis (Oct 29 2020 at 22:21, on Zulip):

(gotta run now)

nikomatsakis (Oct 29 2020 at 22:21, on Zulip):

I'm presuming you want lang team input before you implement

Aaron Hill (Oct 30 2020 at 18:52, on Zulip):

Opened https://github.com/rust-lang/lang-team/issues/66 with just the lint, leaving off the specific details about how to indicate the methods (e.g. #[noop])

Aaron Hill (Oct 30 2020 at 18:53, on Zulip):

let me know if I messed up the produce for opening these kinds of issues

nikomatsakis (Nov 02 2020 at 21:28, on Zulip):

@Aaron Hill ack, that was the wrong sort of proposal, so I missed it

nikomatsakis (Nov 02 2020 at 21:28, on Zulip):

would you object to closing and re-opening a project proposal?

nikomatsakis (Nov 02 2020 at 21:28, on Zulip):

should probably reorganize things a bit

Aaron Hill (Nov 02 2020 at 21:47, on Zulip):

sure

Aaron Hill (Nov 02 2020 at 21:48, on Zulip):

will there really be a 'project group', though?

Aaron Hill (Nov 02 2020 at 21:48, on Zulip):

I think this lint will be very similar to https://github.com/rust-lang/rust/pull/75573 in terms of overall complexity

Aaron Hill (Nov 02 2020 at 21:48, on Zulip):

or have some of the procedurs changed this that PR's FCP?

nikomatsakis (Nov 02 2020 at 21:52, on Zulip):

no there would not be a project group

nikomatsakis (Nov 02 2020 at 21:52, on Zulip):

@Aaron Hill that is one of the renamings I want to do :)

nikomatsakis (Nov 02 2020 at 21:52, on Zulip):

the idea is just to propose "projects", often they just end with "go implement it"

nikomatsakis (Nov 02 2020 at 21:52, on Zulip):

only in rare cases do we need to create a whole group, though we can also create a stream if it's useful for design discussion

Aaron Hill (Nov 02 2020 at 21:53, on Zulip):

sounds good

Aaron Hill (Nov 02 2020 at 22:06, on Zulip):

Opened https://github.com/rust-lang/lang-team/issues/67

Ryan Levick (Dec 16 2020 at 15:33, on Zulip):

@Aaron Hill are you going to work on the implementation for this? This sounds interesting and I would try to take a stab at it if you're busy with other things.

Aaron Hill (Dec 16 2020 at 15:34, on Zulip):

@Ryan Levick I was planning to, but feel free to work on it!

triagebot (Jan 23 2021 at 23:58, on Zulip):

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

rylev (Jan 25 2021 at 11:46, on Zulip):

The implementation for this can be found here: https://github.com/rust-lang/rust/pull/80723

triagebot (Feb 04 2021 at 16:50, on Zulip):

This proposal has been accepted: #375.

Last update: May 07 2021 at 06:30UTC