Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shorten Associated to Assoc and rename Method to AssocFn. #60163

Closed
eddyb opened this issue Apr 22, 2019 · 9 comments
Closed

Shorten Associated to Assoc and rename Method to AssocFn. #60163

eddyb opened this issue Apr 22, 2019 · 9 comments
Labels
A-associated-items Area: Associated items such as associated types and consts. C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Apr 22, 2019

We've been moving away from the "static method" terminology, so IMO we should be more uniform with all associated items, and treat the general case of "methods that may or may not have self" as simply "associated functions".
"Method" terminology would then remain primarily for "method call", i.e. dot syntax for asymmetrically providing the self argument.

Because AssociatedFn is a bit long, I'd find it fitting if we shortened Associated, across rustc, to Assoc, so we end up with AssocFn (which only one letter longer than Method).
Also, when Method appears in the context of associated items already (e.g. {Trait,Impl}ItemKind), it can be just Fn.

cc @rust-lang/compiler (and in particular, I'm curious what @nikomatsakis thinks the decision process for "cosmetic" changes like these should be)

@eddyb eddyb added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels Apr 22, 2019
@nikomatsakis
Copy link
Contributor

I think that discussing terminology like this is probably a good fit for a design meeting. I also think this particular change is a good idea.

@nikomatsakis
Copy link
Contributor

I've added this to my temporary list of design meeting ideas

@agnxy
Copy link
Contributor

agnxy commented May 14, 2019

Hi @eddyb @nikomatsakis I'd like to work on this if the decision has been made.

@oli-obk
Copy link
Contributor

oli-obk commented May 14, 2019

@agnxy one thing that doesn't need a decision is to change all things named Associated... to Assoc.... So you can do that already.

agnxy added a commit to agnxy/rust-clippy that referenced this issue May 25, 2019
This is to fix the breakage introduced by rust-lang/rust#60163.
Centril added a commit to Centril/rust that referenced this issue May 25, 2019
bors added a commit that referenced this issue May 28, 2019
Rename "Associated*" to "Assoc*"

This change is for #60163.

r? @oli-obk
bors added a commit to rust-lang/rust-clippy that referenced this issue May 28, 2019
Rename "Associated*" to "Assoc*"

This is to fix the breakage introduced by rust-lang/rust#60163.

changelog: none
bors added a commit to rust-lang/rust-clippy that referenced this issue May 28, 2019
Rename "Associated*" to "Assoc*"

This is to fix the breakage introduced by rust-lang/rust#60163.

changelog: none
@JohnTitor JohnTitor added the A-associated-items Area: Associated items such as associated types and consts. label Oct 11, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 5, 2020
Centril added a commit to Centril/rust that referenced this issue Mar 6, 2020
Centril added a commit to Centril/rust that referenced this issue Mar 6, 2020
Centril added a commit to Centril/rust that referenced this issue Mar 7, 2020
Centril added a commit to Centril/rust that referenced this issue Mar 12, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 14, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 16, 2020
@eddyb
Copy link
Member Author

eddyb commented Mar 27, 2020

Remaining possible renames:

  • associated -> assoc, Associated -> Assoc:
    /// Maps from an impl/trait `DefId to a list of the `DefId`s of its items.
    query associated_item_def_ids(_: DefId) -> &'tcx [DefId] {}
    /// Maps from a trait item to the trait item "descriptor".
    query associated_item(_: DefId) -> ty::AssocItem {}
    /// Collects the associated items defined on a trait or impl.
    query associated_items(key: DefId) -> &'tcx ty::AssociatedItems {
  • Method -> Fn:

    rust/src/librustc/ty/mod.rs

    Lines 203 to 208 in 7520894

    pub enum AssocKind {
    Const,
    Method,
    OpaqueTy,
    Type,
    }
  • method -> fn:

    rust/src/librustc/ty/mod.rs

    Lines 197 to 199 in 7520894

    /// Whether this is a method with an explicit self
    /// as its first argument, allowing method calls.
    pub method_has_self_argument: bool,

There's also things like rustc_typeck::check::compare_method that could be called something like compare_assoc but ideally it would be clearer that it validates impl assoc items against trait ones, for matching signature/type, where clauses, etc. (cc @nikomatsakis)

@nikomatsakis
Copy link
Contributor

Note: as part of the effort to "extract a shared library for types", I want to do a review of naming conventions for things like this and try to reach some decisions.

@nikomatsakis
Copy link
Contributor

I have mixed feelings about "shortening things" like Assoc. I do appreciate it being concise, but I also think that writing things out has a surprisingly nice effect on things feeling more readable overall.

@nikomatsakis
Copy link
Contributor

But I think that using "associated function" instead of "method" is perhaps wise, particularly if it applies uniformly to things with &self and without.

bors added a commit to rust-lang-ci/rust that referenced this issue Apr 14, 2020