Rework non_local_definitions lint to only use a syntactic heuristic

[Urgau]

This PR reworks the non_local_definitions lint to only use a syntactic heuristic, i.e. not use a type-system logic for whenever an impl is local or not.

Instead the new logic wanted by T-lang in #126768 (comment), which is to consider every paths in Self and Trait and to no longer use the type-system inference trick.

@rustbot labels +L-non_local_definitions
Fixes #126768

[rustbot]

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[BoxyUwU]

r? @WaffleLapkin since you reviewed the original PR

[traviscross]

I looked over the tests; looks right to me.

[workingjubilee]

just typo nits

[bors]

☔ The latest upstream changes (presumably #127493) made this pull request unmergeable. Please resolve the merge conflicts.

[WaffleLapkin]

I don't know how to review this. T-lang's proposal looks to me like "let's make the lint incorrect". And I don't see why the lint is useful, if it's incorrect...

[workingjubilee]

r? lang

[workingjubilee]

cc @rust-lang/lang also posted a followup comment in #126768 (comment)

[traviscross]

Replied with further context and background over in #126768 (comment).

r? @workingjubilee

[workingjubilee]

💭 ...if I asked for lang to review this, and then TC nominated me, does that mean I'm on lang? Is this my punishment for ribbing T-lang one too many times? Am I now the target of my own japes? ...oh no, I'm barely even awake at 9, how am I going to make a meeting at 8?

[traviscross]

Maybe all of that? /s.... but no, seriously that was me just handing back off after answering the lang questions. Looking again, though, I mean to hand it back to compiler, so...

r? compiler

[michaelwoerister]

r? diagnostics

[traviscross]

@Urgau: That sounds right to me, but I'd probably point more normatively to my earlier comments if there were any questions here. And I think @BoxyUwU has a handle on this in any case.

[BoxyUwU]

Sorry for taking so long to get to this, I've been looking over this the past few days and will hopefully be done soon

[bors]

☔ The latest upstream changes (presumably #130724) made this pull request unmergeable. Please resolve the merge conflicts.

[BoxyUwU]

I think the did_has_local_parent/path_has_local_parent logic is written confusingly which is why it took a while to wrap my head around how this file worked. That seems to be pre-existing so I'm fine to land this anyway.

@bors r+

[bors]

📌 Commit 2423e9e has been approved by BoxyUwU

It is now in the queue for this repository.

[bors]

⌛ Testing commit 2423e9e with merge f5cd2c5...

[bors]

☀️ Test successful - checks-actions
Approved by: BoxyUwU
Pushing f5cd2c5 to master...

[rust-timer]

Finished benchmarking commit (f5cd2c5): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.3%	[0.6%, 6.6%]	13
Regressions ❌ (secondary)	0.3%	[0.1%, 0.4%]	3
Improvements ✅ (primary)	-0.7%	[-1.0%, -0.2%]	8
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	3
All ❌✅ (primary)	1.8%	[-1.0%, 6.6%]	21

Max RSS (memory usage)

Results (primary 0.4%, secondary -3.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.3%	[2.3%, 2.3%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.4%	[-1.4%, -1.4%]	1
Improvements ✅ (secondary)	-3.3%	[-3.3%, -3.3%]	1
All ❌✅ (primary)	0.4%	[-1.4%, 2.3%]	2

Cycles

Results (primary 1.8%, secondary 1.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.9%	[0.8%, 4.3%]	8
Regressions ❌ (secondary)	1.9%	[1.9%, 1.9%]	1
Improvements ✅ (primary)	-0.6%	[-0.7%, -0.5%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.8%	[-0.7%, 4.3%]	12

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 768.261s -> 767.582s (-0.09%)
Artifact size: 341.50 MiB -> 341.38 MiB (-0.03%)

[Urgau]

Same as last times (#120393 (comment), #125722 (comment)), the version of diesel used by rustc-perf is too old and use named const AA items instead of un-named const _ items, making the lint trigger many times (more than 150 times last time). We ruled in the past that given the regression where located in one benchmark only, and only a pathological case that the regressions where fine. Marking as triaged.

@rustbot label: +perf-regression-triaged
cc rust-lang/rustc-perf#1819