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

Add RFC for explicit self member access. #173

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jemc
Copy link
Member

@jemc jemc commented May 25, 2020

This PR adds my RFC for explicit self member access.

Note: I have also filed another RFC that builds on this one

@jemc jemc self-assigned this May 25, 2020
@jemc jemc mentioned this pull request May 25, 2020
@sylvanc
Copy link
Contributor

sylvanc commented May 26, 2020

One possibility would be to combine explicit this.f notation with an explicit this in function signatures, i.e. changing fun box foo() to fun foo(this: box).

@aturley
Copy link
Member

aturley commented May 26, 2020

Let's talk about this again next week.

@Theodus Theodus added the status - new The RFC is new and ready for discussion. label Jun 14, 2020
@aturley
Copy link
Member

aturley commented Jun 16, 2020

Discuss more next week.

@jkankiewicz
Copy link

jkankiewicz commented Jul 20, 2020

One possibility would be...an explicit this in function signatures, i.e. changing fun box foo() to fun foo(this: box).

To quote the "Zen of Python", "Explicit is better than implicit."

For those who might think it shouldn't be necessary to specify an argument whose name is predetermined, using a name other than self for the object reference in a Python method's signature is considered erroneous even though it's not a keyword.

@sylvanc's proposal would even improve C++ because the arity of a member function would always match its declaration's whether it's static or not and the placement of a const qualifier on a member function would be obvious e.g.

class Wombat
{
public:
  std::optional<std::string> name;

private:
  uint64_t _hunger_level = 0;

public:
  Wombat(auto* this, const std::string& name_);  // vs. Wombat(const std::string& name_);

  static Wombat hungry(const std::string& name_, const unit64_t hunger_);  // no change

  uint64_t hunger(const auto* this);  // vs. uint64_t hunger() const;

  uint64_t set_hunger(auto* this, const uint64_t to = 0); // vs. uint64_t set_hunger(const uint64_t to = 0);

};

@jemc
Copy link
Member Author

jemc commented Jul 21, 2020

Personally, I don't treat "explicit is better than implicit" as a valid argument on its own in most cases, as it can be use to justify a great deal of proposals that we could probably all agree aren't desirable for the language (e.g. "the language should require us to explicitly specify which assembly registers to use for our function parameters").

There are always things that are implicit in your semantic model (that's just the nature of abstraction), and it wouldn't be "better" to make all of those things explicit.

Perhaps a better formulation would be "unsurprising is better than surprising", as this way of stating it acknowledges explicitly (oh, the irony!) the implicit premise that you expect your users to have a certain base level of understanding, and you should make explicit anything that would be surprising/unexpected given that base of understanding. And this gets you to the point of debating the real issue - which is deciding exactly what base level of understanding should be expected, allowing you the option of improving your learning materials to help users meet that base level rather than rejecting every proposal that might expect them to learn more.


Note that this is not a comment about Sylvan's proposal, but just me explaining why I tend to dismiss the "explicit is better than implicit" line of argument - because it isn't specific or explicit enough to be good advice in every case in which people may try to apply it.

Base automatically changed from master to main February 8, 2021 22:15
@SeanTAllen SeanTAllen added the discuss during sync Should be discussed during an upcoming sync label Feb 9, 2022
@SeanTAllen
Copy link
Member

I'm going to try doing a modification to a source file and see how I feel about this.. I'm not a fan of @ as it breaks FFI (if it didnt, I would be fine with it). I would be interested in hearing conversation on other symbols to use instead of @.

@rhagenson
Copy link
Member

The general consensus I heard during sync today was that everyone preferred "explicit over implicit" -- myself included, but not verbalized. I am personally in favor of using some symbol for member access, if not @ then perhaps some other symbol. I do not have a preference for what symbol to use, but would prefer some "wide" symbol rather than a "narrow" symbol like : -- wider symbols are easier to read without syntax highlighting so generally better when the goal is to rapidly differentiate @field from field.

My preference is for a symbol rather than explicit this or explicit receiver A.method(this: A, x: B, ...) due to the presence of #174 which should have mirroring syntax. A.method(@x: B, ...) is far cleaner to me than A.method(this.x: B, ...).

@SeanTAllen
Copy link
Member

SeanTAllen commented Feb 23, 2022

I'm wary of a symbol for "self" access. Especially as we already have this.

I tried out this. on a couple different bits of code I had written and it wasn't bad at all. I can see some instances where it would be, but I'm not sure if a symbol would make it better.

I'm not coming out against a symbol, but I am leaning away from it. I would definitely prefer if there is a symbol to not make all FFI using code have to change as well. I'd rather introduce a new symbol but I don't really see any available except # which I was hoping to use to bring the compile time expressions from ponyta into Pony using.

@SeanTAllen
Copy link
Member

I appear to be an incrementalist at the moment. I find myself not in favor of adding a new symbol at this time. And rather only want consider the explicit self member access which effectively creates a new named scope.

@ergl on the last sync call raised the important point about the intersection of explicit self member access + named parameters. It's true that with named arguments using where that parameter names can conflict with field of method names and result in breaking changes from feeling "forced to rename a parameter".

For me, I'd like to resolve the named argument issue (perhaps via explicit self member access and requiring this for field and function access) (or doing away with named arguments)(or something else).

Then I'd also like to resolve the issue that I feel #174 addresses which is what can become an incredibly verbose (but usually fine) field initialization with object that have tons of fields like those seen in the GitHub Rest API.

I'm happy to resolve them in either order, but I would definitely like to consider options outside the current context of the two RFCs.

For example, perhaps the overly verbose and occasionally bug inducing issues that I feel #174 is an attempt to address would be better resolved via having a macro system.

I feel that both RFCs from @jemc, #173 and #174 are addressing import points but, I want to consider other alternatives as well.

@SeanTAllen
Copy link
Member

I have a question for folks, please respond directly on this comment with thumbs up/thumbs down reaction (and not adding additional comments for now that bust up the flow of conversation here).

One way to address the "where" problem with named arguments is to have parameters "live in a different namespace" from fields and methods. The approach we've discussed so far has only been "change rules to access fields and methods".

How would folks feel if instead of a required this or whatnot for fields and methods, it was instead a keyword to access parameters? As a total strawman to explain, not to set as the final idea:

fun double(x: U64): U64 =>
  param.x * 2

I'm dubious myself (for a number of reasons), but I want to explore all ideas for "parameters are in a different namespace from fields and methods".

@ergl
Copy link
Member

ergl commented Mar 1, 2022

One option covered in the sync call is to require the this qualifier only when not having it would be ambiguous.

The advantage of this would be that it would be a backwards-compatible change:

class Foo
  let x: U8 = 0

  fun set_x(x: U8) => x = x // ambiguous, needs this.x

  fun add_x(delta: U8) => x = x + delta // not ambiguous, this. not needed

@ergl ergl removed the discuss during sync Should be discussed during an upcoming sync label Mar 1, 2022
@SeanTAllen
Copy link
Member

I tried out explicit this in some additional code and I'm good with it. The additional amount of characters turned out to not be that much and made it somewhat easier to follow code (barely though on that, I think it might be nice when doing "maintenance programming). I'm also ok with the status quo.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 3, 2023
@SeanTAllen
Copy link
Member

We discussed at sync and for now, we can't come to an agreement on this so we are leaving behind for now.

If we are making a very large breaking change then we should revisit as we are more likely to find agreement on this. In particular, if there was a very large breaking change, I would be more open to the @ breaking change in this and so, we might then be able to reach agreement.

@SeanTAllen SeanTAllen removed status - new The RFC is new and ready for discussion. discuss during sync Should be discussed during an upcoming sync labels Jan 3, 2023
@ponylang-main ponylang-main added discuss during sync Should be discussed during an upcoming sync status - new The RFC is new and ready for discussion. labels Jan 3, 2023
@SeanTAllen SeanTAllen removed the status - new The RFC is new and ready for discussion. label Jan 3, 2023
@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Jan 3, 2023
@adrianboyko
Copy link

I propose that .foo be equivalent to this.foo, i.e. that the default receiver is always this.

Example:

  • if .foo == 42 then bar()
  • new val create(.name, .return_type, .params, .partial, .guard, .attachments) => None

Pros:

  • Doesn't break FFI by repurposing @
  • Doesn't introduce a new symbol into the language.
  • Doesn't introduce new "no-period member access" syntax (i.e. @foo discards the .)
  • It's teachable: "What if there's no object specified before the period? The default is this!"
  • It's an unsurprising default -- what else would the default object be, other than this?

Cons:

  • . isn't a "wide" or "highly visible" character, but nobody ever looks at origin.x and wonders what "originx" means, so I think periods are visible enough.

Notes:

  • For consistency, .> should also default to receiver this
  • Since locals and params cannot begin with _, any name starting with _ and a lowercase letter is (afaik) an unambiguous reference to a member. Maybe _foo can be sugared to this._foo, avoiding the need for ._foo?

Skip this next part if you've already read enough -- this is way out on a tangent:

  • For consistency, the partial application syntax should probably be foo.~addmul(3). I.e. member access should always be through a period, but the period can sometimes be followed by a modifier. So .> means "give me the member method but wrap it so that it always returns self", and .~ would mean "give me the member method but wrap it in a partial application."
  • .~ should default to receiver this, just like . and .> should. It looks like let x = ~addmul(4) is not currently valid Pony, but let x = .~addmul(4) should be.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 4, 2023
@jemc
Copy link
Member Author

jemc commented Jan 10, 2023

We discussed @adrianboyko's proposal comment above in today's sync call - that idea has parsing ambiguity issues, including in an example like this:

.foo()
.bar()

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Jan 10, 2023
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 16, 2023
@jemc jemc removed the discuss during sync Should be discussed during an upcoming sync label Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.