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

Rename me -> self #1382

Merged
merged 22 commits into from
Dec 6, 2022
Merged

Rename me -> self #1382

merged 22 commits into from
Dec 6, 2022

Conversation

josh11b
Copy link
Contributor

@josh11b josh11b commented Jul 12, 2022

Carbon has been using a me: Self or addr me: Self* deduced parameter to mark a function as a method as decided in #494 and implemented in #722 . This does not match existing languages, and so this proposal switches me to self to match Python, Rust, and Swift.

@josh11b josh11b added the proposal A proposal label Jul 12, 2022
@josh11b josh11b requested a review from a team July 12, 2022 15:44
Comment on lines 82 to 85
We could stay with the status quo, though as the chart in
[the background section](#background) shows other languages have found it
acceptable to require a 4 character explicit keyword for accessing members of
the current object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do any of those languages have as much syntactic overhead for declaring a method as Carbon does? I worry not only that [self: Self] is longer than [me: Self] but also that it looks more redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is what it looks like in the parameter list for languages that have explicit member access:

Python: self,
Javascript and TypeScript: `` --implicit--
Rust: self is an abbreviation for `self: Self` doc

I think if we are concerned about the verbosity in the method declaration, a better fix is to use Rust's approach.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a discussion for reducing the verbosity in the method declaration: #1777

Copy link

@PeterWrighten PeterWrighten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that you guy misunderstand...carbon doesn't have self keyword, so Me is just a naming style, it doesn't matter...

@josh11b
Copy link
Contributor Author

josh11b commented Jul 23, 2022

Seems that you guy misunderstand...carbon doesn't have self keyword, so Me is just a naming style, it doesn't matter...

I have added self to the list of keywords in this proposal.

@josh11b josh11b marked this pull request as ready for review July 23, 2022 16:41
@github-actions github-actions bot added the proposal rfc Proposal with request-for-comment sent out label Jul 23, 2022
@cjdb
Copy link
Contributor

cjdb commented Jul 23, 2022

I think a strong argument in favour of this proposal is that it ensures that the type and value have the same name, which seems to be an industry standard when both exist.

Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might consider splitting all the misc file changes to a separate PR in order to make it simpler to get feedback.

Comment on lines 105 to 108
We could also switch to `this`, primarily to benefit
[C++ users](https://en.cppreference.com/w/cpp/language/this). However, we are
worried that it frequently not being a pointer would be surprising to those C++
users.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this rationale should be expanded because of the break in consistency with C++. Yes, it won't always be the same type: but most C++ developers are familiar with the basic meaning of this, and the type would be in the function signature.

For example, although you don't note the rationale here, I think you'd noted that self operates this way in Rust. However, that's Rust, and most C++ developers shouldn't be expected to be familiar with it. The surrounding syntax also differs from Rust, which I would expect to hamper experienced Rust devs.

As a counterpoint to the Rust type argument, more likely C++ developers would be familiar with self in Python, due to its popularity, and it's always the same type there. From what I can tell, Swift is similar. Thus I don't see self as offering better cross-language consistency from a type perspective, since Rust is in a minority.

I believe more information should be provided for the leads here -- it may even be appropriate to raise the question for explicit resolution, although it's perhaps redundant with this proposal.

Copy link
Contributor

@Pixep Pixep Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the 'cons' colums, from a semantic standpoint, it feels less clear that this refers to the current instance. self feels like it is less ambiguously refer 'itself' vs designating something with 'this'/'that'.

Copy link

@emlai emlai Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • we are worried that it frequently not being a pointer would be surprising to those C++ users

    Could expand this alternative to also suggest relying on user feedback on whether this not being a pointer would actually be surprising, rather than assumption.

  • This applies to pretty much all syntax changes from C++, but: could mention the language strangeness budget i.e. the balancing of seemingly small syntax changes like this versus the more major changes we're planning to make.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other point that may be worth capturing here is that we view it as an advantage to use the same spelling for the variable self as for the type Self, and while this might make an acceptable name for the object parameter, Self is much more strongly established as the name for the current class, for example in PL research, and there is no precedent for a type named This.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've expanded in this update: fda1ceb

Copy link
Contributor

@micttyl micttyl Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(research is already done in the proposal)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added that link to history in the background section: see 2b7ee5a

@josh11b
Copy link
Contributor Author

josh11b commented Jul 23, 2022

You might consider splitting all the misc file changes to a separate PR in order to make it simpler to get feedback.

Okay I will figure out how to do that.

@josh11b
Copy link
Contributor Author

josh11b commented Jul 23, 2022

You might consider splitting all the misc file changes to a separate PR in order to make it simpler to get feedback.

Okay I will figure out how to do that.

Moved implementation of proposal to #1624

@vbregier
Copy link

vbregier commented Jul 30, 2022

Is it really necessary to have a keyword here ?
Python is cited here, but in python, self is not a keyword. Any name can be used as first argument to make a method.

In carbon, the rule could be that a function within class that has first argument of type Self is a method.
This leaves it to the programmer to choose which argument name is better suited, and makes one fewer keywords in the language.

@jonmeow
Copy link
Contributor

jonmeow commented Jul 30, 2022

Is it really necessary to have a keyword here ? Python is cited here, but in python, self is not a keyword. Any name can be used as first argument to make a method.

In carbon, the rule could be that a function within class that has first argument of type Self is a method. This leaves it to the programmer to choose which argument name is better suited, and makes one fewer keywords in the language.

Making the receiver type has already been considered:
https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p0722.md#full-receiver-type

When we review proposals, we try not to rehash decided issues. Since there is a decision there will be a receiver type, and this PR is about the name, please don't continue this thread here: it's off-topic.

That said, I don't see any recent issue asking the leads about removing the receiver type. If you want to suggest a change to remove the receiver type, a good way to start is by creating a new issue, putting it under the "Issues for leads" project, and providing a rationale for making it optional that offers new information not previously considered. In the FAQ, this section has some pertinent notes on revisiting decisions.

@emlai
Copy link

emlai commented Jul 30, 2022

There's a discussion about the receiver type notation: #1777. It could be turned into an issue for leads or a proposal, at some point. Might need some fleshing out though.

@josh11b josh11b closed this Aug 24, 2022
@josh11b josh11b deleted the self branch August 24, 2022 22:46
@josh11b josh11b restored the self branch August 24, 2022 22:46
@josh11b josh11b reopened this Aug 24, 2022
@zygoloid
Copy link
Contributor

For me, the most convincing reason to change the spelling of me to self would be to support shorthand syntax [self] and [addr self], which don't really make so much sense when the value and the type are spelled differently. And the most significant reason to not make this change is because in the absence of a shorthand syntax, it makes the method introduction syntax unpalatably long. So I see the change in spelling of me -> self and the addition of a shorthand syntax as being coupled: I'd want us to do either neither or both.

Would it be reasonable to also add the shorthand syntax in this proposal?

@josh11b
Copy link
Contributor Author

josh11b commented Sep 15, 2022

For me, the most convincing reason to change the spelling of me to self would be to support shorthand syntax [self] and [addr self], which don't really make so much sense when the value and the type are spelled differently. And the most significant reason to not make this change is because in the absence of a shorthand syntax, it makes the method introduction syntax unpalatably long. So I see the change in spelling of me -> self and the addition of a shorthand syntax as being coupled: I'd want us to do either neither or both.

Would it be reasonable to also add the shorthand syntax in this proposal?

@chandlerc How would you feel about this proposal with the shorthand syntax included?

@chandlerc
Copy link
Contributor

For me, the most convincing reason to change the spelling of me to self would be to support shorthand syntax [self] and [addr self], which don't really make so much sense when the value and the type are spelled differently. And the most significant reason to not make this change is because in the absence of a shorthand syntax, it makes the method introduction syntax unpalatably long. So I see the change in spelling of me -> self and the addition of a shorthand syntax as being coupled: I'd want us to do either neither or both.
Would it be reasonable to also add the shorthand syntax in this proposal?

@chandlerc How would you feel about this proposal with the shorthand syntax included?

I would prefer to revisit the syntax holistically, and once we have #1974 in the design. Before that, I would prefer to not add a shorthand syntax -- I think it forces us to think through implications of the shorthand syntax without much benefit as we'll have to do that again if we do a more holistic revisit.

I don't feel strongly about the 2 character difference between me and self. I think with either name, the syntax is too long -- addr self: Self* is 16 characters instead of 14 characters of today's addr me: Self*.

I think the much easier thing to decouple from a holistic revisit of the syntax, including shorthand syntax, is the name. That is also the part that I think causes much more confusion when discussing and writing examples. It also impacts the body of code, not just the signature, and so is a bit more annoying to update later. So overall, I'd prefer to go ahead and switch to self now, but not spend time trying to come up with a shorthand syntax.

@chandlerc
Copy link
Contributor

@zygoloid - Any thoughts on my comments?

@zygoloid
Copy link
Contributor

zygoloid commented Dec 6, 2022

@zygoloid - Any thoughts on my comments?

Let's move forward with this proposal and see if we get negative feedback on verbosity, and look at handling any such feedback once we have that experience.

Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@josh11b josh11b merged commit d1210c4 into carbon-language:trunk Dec 6, 2022
@josh11b josh11b deleted the self branch December 6, 2022 23:58
josh11b added a commit that referenced this pull request Dec 7, 2022
Implements change proposed in #1382

Replaces #1624
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal rfc Proposal with request-for-comment sent out proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.