diagnostics: if AssocFn has self argument, describe as method#108324

notriddle · 2023-02-21T21:22:01Z

Discussed in https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/.22associated.20function.22.20vs.20.22method.22/near/329265515

This commit also changes the tooltips on rustdoc intra-doc links targeting methods.

For anyone not sure why this is being done, see the Reference definitions of these terms in https://doc.rust-lang.org/1.67.1/reference/items/associated-items.html#methods

Associated functions whose first parameter is named self are called methods and may be invoked using the method call operator, for example, x.foo(), as well as the usual function call notation.

In particular, while this means it's technically correct for rustc to refer to a method as an associated function (and there are a few cases where it'll still do so), rustc must never use the term "method" to refer to an associated function that does not have a self parameter.

rustbot · 2023-02-21T21:22:08Z

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-02-21T21:22:13Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in need_type_info.rs

cc @lcnr

compiler-errors

I'm a fan of this change, but maybe cc @rust-lang/wg-diagnostics on this, since it kinda solidifies our usage of some terminology that has been up in the air in the past.

r=me on the impl modulo one nit

src/tools/clippy/clippy_lints/src/missing_doc.rs

rustbot · 2023-02-22T15:24:24Z

Some changes occurred in src/tools/cargo

cc @ehuss

Discussed in https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/.22associated.20function.22.20vs.20.22method.22/near/329265515 This commit also changes the tooltips on rustdoc intra-doc links targeting methods.

…thod

compiler-errors · 2023-02-22T16:54:45Z

@bors r+

bors · 2023-02-22T16:54:48Z

📌 Commit bf9b980 has been approved by compiler-errors

It is now in the queue for this repository.

compiler-errors · 2023-02-22T18:45:56Z

@bors p=1

given the number of files this touches, this may be bitrotty.

compiler-errors · 2023-02-22T19:27:34Z

@bors r- r=compiler-errors,davidtwco,estebank,oli-obk

bors · 2023-02-22T19:28:02Z

📌 Commit bf9b980 has been approved by compiler-errors,davidtwco,estebank,oli-obk

It is now in the queue for this repository.

bors · 2023-02-23T00:19:15Z

⌛ Testing commit bf9b980 with merge 0978711...

bors · 2023-02-23T03:11:37Z

☀️ Test successful - checks-actions
Approved by: compiler-errors,davidtwco,estebank,oli-obk
Pushing 0978711 to master...

rust-timer · 2023-02-23T10:08:45Z

Finished benchmarking commit (0978711): 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)	1.1%	[1.0%, 1.2%]	2
Regressions ❌ (secondary)	2.2%	[2.0%, 2.6%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.3%	[-2.3%, -2.3%]	1
All ❌✅ (primary)	1.1%	[1.0%, 1.2%]	2

Max RSS (memory usage)

Results

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)	-	-	0
Regressions ❌ (secondary)	2.5%	[2.5%, 2.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

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

nnethercote · 2023-02-24T00:38:17Z

Noise.

@rustbot label: +perf-regression-triaged

… r=compiler-errors,davidtwco,estebank,oli-obk diagnostics: if AssocFn has self argument, describe as method Discussed in https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/.22associated.20function.22.20vs.20.22method.22/near/329265515 This commit also changes the tooltips on rustdoc intra-doc links targeting methods. For anyone not sure why this is being done, see the Reference definitions of these terms in <https://doc.rust-lang.org/1.67.1/reference/items/associated-items.html#methods> > Associated functions whose first parameter is named `self` are called methods and may be invoked using the [method call operator](https://doc.rust-lang.org/1.67.1/reference/expressions/method-call-expr.html), for example, `x.foo()`, as well as the usual function call notation. In particular, while this means it's technically correct for rustc to refer to a method as an associated function (and there are a few cases where it'll still do so), rustc *must never* use the term "method" to refer to an associated function that does not have a `self` parameter.

…escr, r=compiler-errors diagnostics: avoid querying `associated_item` in the resolver Fixes rust-lang#108529 CC rust-lang#108324

rustbot assigned davidtwco Feb 21, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 21, 2023

This comment has been minimized.

Sign in to view

notriddle force-pushed the notriddle/assoc-fn-method branch from 9069bf9 to c0862f1 Compare February 21, 2023 22:19

compiler-errors reviewed Feb 22, 2023

View reviewed changes

src/tools/clippy/clippy_lints/src/missing_doc.rs Outdated Show resolved Hide resolved

oli-obk approved these changes Feb 22, 2023

View reviewed changes

davidtwco approved these changes Feb 22, 2023

View reviewed changes

notriddle force-pushed the notriddle/assoc-fn-method branch from c0862f1 to 904ebd8 Compare February 22, 2023 15:24

notriddle force-pushed the notriddle/assoc-fn-method branch from 904ebd8 to 6c1d9b9 Compare February 22, 2023 15:26

notriddle added 4 commits February 22, 2023 08:40

diagnostics: update test cases to refer to assoc fn with self as me…

3f37412

…thod

rustdoc: update test case with intra-doc link pointing to method

881280f

clippy: update clippy to use new TyCtxt::def_descr

bf9b980

notriddle force-pushed the notriddle/assoc-fn-method branch from 6c1d9b9 to bf9b980 Compare February 22, 2023 15:40

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2023

estebank approved these changes Feb 22, 2023

View reviewed changes

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 22, 2023

bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 22, 2023

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 22, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 23, 2023

bors merged commit 0978711 into rust-lang:master Feb 23, 2023

rustbot added this to the 1.69.0 milestone Feb 23, 2023

bors mentioned this pull request Feb 23, 2023

rustdoc: reduce allocations when generating tooltips #108098

Merged

notriddle deleted the notriddle/assoc-fn-method branch February 23, 2023 03:56

xFrednet mentioned this pull request Feb 23, 2023

Bump rust nightly -> 2023-03-09 rust-marker/marker#101

Closed

rustbot added the perf-regression Performance regression. label Feb 23, 2023

rustbot added the perf-regression-triaged The performance regression has been triaged. label Feb 24, 2023

clubby789 mentioned this pull request Feb 27, 2023

ICE: None in query system #108529

Closed

notriddle mentioned this pull request Feb 27, 2023

diagnostics: avoid querying associated_item in the resolver #108533

Merged

Y-Nak mentioned this pull request Apr 22, 2023

Change the constructor of salsa::Id to const fn salsa-rs/salsa#440

Merged

Uh oh!

Conversation

notriddle commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 21, 2023

Uh oh!

rustbot commented Feb 21, 2023

Uh oh!

This comment has been minimized.

compiler-errors left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rustbot commented Feb 22, 2023

Uh oh!

compiler-errors commented Feb 22, 2023

Uh oh!

bors commented Feb 22, 2023

Uh oh!

compiler-errors commented Feb 22, 2023

Uh oh!

compiler-errors commented Feb 22, 2023

Uh oh!

bors commented Feb 22, 2023

Uh oh!

bors commented Feb 23, 2023

Uh oh!

bors commented Feb 23, 2023

Uh oh!

rust-timer commented Feb 23, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Uh oh!

nnethercote commented Feb 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

notriddle commented Feb 21, 2023 •

edited

Loading

compiler-errors left a comment •

edited

Loading