Change "method" to "associated function" by mark-i-m · Pull Request #69498 · rust-lang/rust

mark-i-m · 2020-02-26T22:52:37Z

I'm opening this mostly as a test to see what the diagnostic changes would be. It seems that this makes them somewhat more verbose, and I'm not sure it's worth it...

The relevant changes are the last two commits (it is rebased on top of #67742)

eddyb · 2020-02-26T22:55:32Z

Can you open a PR to fix #60163 without changing diagnostics?
Then we can separately do the diagnostic changes.

bjorn3 · 2020-02-28T20:34:46Z

To me a method is a special kind of associated function which takes a self parameter.

mark-i-m · 2020-03-03T17:47:59Z

@bjorn3 I agree, but the compiler doesn't currently make a distinction iiuc

bjorn3 · 2020-03-03T18:03:16Z

It would be nice if the error messages do make the distinction.

mark-i-m · 2020-03-03T18:55:52Z

Rebased over #69674

@rustbot modify labels: -S-waiting-on-review +S-blocked

Centril · 2020-03-03T21:37:27Z

It would be nice if the error messages do make the distinction.

I think error messages should only make the distinction where it actually matters. Otherwise, you have to scan for self in many places where it is semantically irrelevant whether self is there or not.

@eddyb

Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

@eddyb

Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

@eddyb

Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

@eddyb

Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

bors · 2020-03-08T14:54:47Z

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

@eddyb

Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

mark-i-m · 2020-03-12T20:48:20Z

@rustbot modify labels: +S-waiting-on-review -S-blocked

matthewjasper · 2020-03-13T21:26:03Z

@bors r+
cc @rust-lang/wg-diagnostics

bors · 2020-03-13T21:26:05Z

📌 Commit b6518f0 has been approved by matthewjasper

@matthewjasper

…sper Change "method" to "associated function" r? @matthewjasper cc @Centril @eddyb rust-lang#67742 I'm opening this mostly as a test to see what the diagnostic changes would be. It seems that this makes them somewhat more verbose, and I'm not sure it's worth it... The relevant changes are the last two commits (it is rebased on top of rust-lang#67742)

@matthewjasper

…sper Change "method" to "associated function" r? @matthewjasper cc @Centril @eddyb rust-lang#67742 I'm opening this mostly as a test to see what the diagnostic changes would be. It seems that this makes them somewhat more verbose, and I'm not sure it's worth it... The relevant changes are the last two commits (it is rebased on top of rust-lang#67742)

@ghost

Rollup of 7 pull requests Successful merges: - #69357 (Emit 1-based column numbers in debuginfo) - #69471 (Remove `sip::Hasher::short_write`.) - #69498 (Change "method" to "associated function") - #69967 (Remove a few `Rc`s from RegionInferenceCtxt) - #69987 (Add self to .mailmap) - #69991 (fix E0117 message out of sync) - #69993 (Add long error explanation for E0693) Failed merges: r? @ghost

@ghost

Rollup of 7 pull requests Successful merges: - #69357 (Emit 1-based column numbers in debuginfo) - #69471 (Remove `sip::Hasher::short_write`.) - #69498 (Change "method" to "associated function") - #69967 (Remove a few `Rc`s from RegionInferenceCtxt) - #69987 (Add self to .mailmap) - #69991 (fix E0117 message out of sync) - #69993 (Add long error explanation for E0693) Failed merges: r? @ghost

estebank · 2020-03-15T18:26:33Z

It would be nice if the error messages do make the distinction.
I think error messages should only make the distinction where it actually matters. Otherwise, you have to scan for self in many places where it is semantically irrelevant whether self is there or not.

I disagree, I think it makes it much easier to teach the concepts if we are consistent in calling associated functions with self parameters "methods". I've seen this confusion play out many times and it is our responsibility to reduce "induced" confusion.

mark-i-m · 2020-03-15T18:30:26Z

I guess I've personally experienced the opposite... in this case, I always used to here "associated X" and not really know what it meant because e.g. associated fn where inconsistently referred to as methods, functions, or assoc functions. Picking one and sticking to it would be good.

On the other hand, I think the main point in favor of method is that it is the terminology used by most other languages (e.g. java, c++).

Centril · 2020-03-15T18:33:33Z

Java also has the concept of "static method", which is our notion of an associated method without self, so that adds more to the confusion. Meanwhile, Haskell calls any function in a type class "method".

@eddyb

Replace some desc logic in librustc_lint with article_and_desc r? @eddyb @Centril @matthewjasper Followup to rust-lang#69674 Blocked on rust-lang#69498

@eddyb

Replace some desc logic in librustc_lint with article_and_desc r? @eddyb @Centril @matthewjasper Followup to rust-lang#69674 Blocked on rust-lang#69498

@eddyb

Replace some desc logic in librustc_lint with article_and_desc r? @eddyb @Centril @matthewjasper Followup to rust-lang#69674 Blocked on rust-lang#69498

rust-highfive assigned matthewjasper Feb 26, 2020

Centril reviewed Feb 26, 2020
View reviewed changes

Comment threadsrc/librustc/hir/map/mod.rs Outdated

eddyb reviewed Feb 26, 2020
View reviewed changes

Comment threadsrc/librustc_hir/def.rs Outdated

Centril reviewed Feb 26, 2020
View reviewed changes

Comment threadsrc/test/ui/suggestions/suggest-methods.stderr Outdated

mark-i-m mentioned this pull request Mar 3, 2020
Rename DefKind::Method and TraitItemKind::Method #69674
Merged

mark-i-m force-pushed the describe-it-2 branch from 90d7531 to 1866043Compare March 3, 2020 18:55

rustbot added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 3, 2020

mark-i-m force-pushed the describe-it-2 branch from 1866043 to 99586dbCompare March 5, 2020 03:03

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 5, 2020
Rollup merge ofrust-lang#69674- mark-i-m:assoc-fn, r=Centril
210d693
Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

mark-i-m mentioned this pull request Mar 5, 2020
Replace some desc logic in librustc_lint with article_and_desc #69740
Merged

Centril added a commit to Centril/rust that referenced this pull request Mar 6, 2020
Rollup merge ofrust-lang#69674- mark-i-m:assoc-fn, r=Centril
19272a0
Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

Centril added a commit to Centril/rust that referenced this pull request Mar 6, 2020
Rollup merge ofrust-lang#69674- mark-i-m:assoc-fn, r=Centril
ed6ecf6
Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

Centril added a commit to Centril/rust that referenced this pull request Mar 7, 2020
Rollup merge ofrust-lang#69674- mark-i-m:assoc-fn, r=Centril
e3def0e
Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

Centril added a commit to Centril/rust that referenced this pull request Mar 12, 2020
Rollup merge ofrust-lang#69674- mark-i-m:assoc-fn, r=matthewjasper
9bc7386
Rename DefKind::Method and TraitItemKind::Method r? @eddyb, @Centril, or @matthewjasper cc rust-lang#69498 rust-lang#60163

mark-i-m added 2 commits March 12, 2020 15:46

change method -> associated function
c8b527e

update tests
b6518f0

mark-i-m force-pushed the describe-it-2 branch from 99586db to b6518f0Compare March 12, 2020 20:48

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Mar 12, 2020

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 13, 2020

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 13, 2020

JohnTitor mentioned this pull request Mar 14, 2020
Rollup of 8 pull requests #69996
Closed

Dylan-DPC-zz mentioned this pull request Mar 14, 2020
Rollup of 7 pull requests #70008
Closed

Dylan-DPC-zz mentioned this pull request Mar 15, 2020
Rollup of 7 pull requests #70016
Merged

bors merged commit f40272c into rust-lang:masterMar 15, 2020

mark-i-m deleted the describe-it-2 branch March 15, 2020 16:16

Uh oh!

Uh oh!

Conversation

mark-i-m commented Feb 26, 2020

Uh oh!

eddyb commented Feb 26, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bjorn3 commented Feb 28, 2020

Uh oh!

mark-i-m commented Mar 3, 2020

Uh oh!

bjorn3 commented Mar 3, 2020

Uh oh!

mark-i-m commented Mar 3, 2020

Uh oh!

Centril commented Mar 3, 2020

Uh oh!

bors commented Mar 8, 2020

Uh oh!

mark-i-m commented Mar 12, 2020

Uh oh!

matthewjasper commented Mar 13, 2020

Uh oh!

bors commented Mar 13, 2020

Uh oh!

estebank commented Mar 15, 2020

Uh oh!

mark-i-m commented Mar 15, 2020

Uh oh!

Centril commented Mar 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants