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

Self access for abstracts #62

Merged
merged 4 commits into from
Nov 15, 2021
Merged

Conversation

markknol
Copy link
Member

@markknol markknol commented Jun 19, 2019

Add a way to access "self" for abstracts, which is a getter for (cast this:MyAbstract).

abstract MyAbstract(MyType) as <ident> { 
}

Rendered version

@nadako
Copy link
Member

nadako commented Jun 19, 2019

I like the idea and I can totally relate to the use-cases. One thing I'm not sure about is whether we should actually generate a getter field. I would probably go with adding an identifier resolution rule instead.

@markknol
Copy link
Member Author

I'm obviously no compiler developer so it doesn't have to be a actual field. As long as it works that way.
What's a better way to describe that particular section?

@nadako
Copy link
Member

nadako commented Jun 19, 2019

I'm obviously no compiler developer so it doesn't have to be a actual field. As long as it works that way.
What's a better way to describe that particular section?

I mean, that is also an option, I'm just proposing an alternative that seems a bit more clean to me (however it might be more dirty implementation-wise). If you want to also add this to the proposal, then I'd say something like:

Alternatively there could be a new identifier resolution rule:

  • When resolving and identifier while typing a non-static abstract method, consider the name, provided with as <ident>.

Although this raises more questions, as in at what place in the resolution order should it be considered, can it be overshadowed by local vars, should there be an error when declaring an argument/local var/field with the same name, etc.

@RealyUniqueName
Copy link
Member

RealyUniqueName commented Jun 19, 2019

I think it should behave the same way as a field, but generate the same syntax tree as implicit cast of from Type.

@skial skial mentioned this pull request Jun 20, 2019
1 task
- Leave out suggested "design", leave it up to the compiler team
- add duplication error example
- (impr/m)ove some text
@Simn
Copy link
Member

Simn commented Sep 11, 2019

This is a good idea.

I have a question about the syntax though: Shouldn't the as be part of the underlying type? For instance, abstract Vector2d(Vector2dImpl as self). The reason is that we're referring to the underlying type, not the abstract. This would also avoid a special case for @:coreType abstracts where we have no underlying type. Never mind I'm retarded.

I think it should behave the same way as a field, but generate the same syntax tree as implicit cast of from Type.

I agree.

@benmerckx
Copy link
Contributor

Isn't this referring to the underlying type, whereas self or whichever identifier would refer to it typed as the abstract?

@Simn
Copy link
Member

Simn commented Sep 11, 2019

You're right, I have no idea what I was thinking.

@haxiomic
Copy link
Member

haxiomic commented Sep 11, 2019

I like the concept well enough but just a nitpick: I struggle with the as <ident> syntax – It took me a while to figure out what this meant. I associate as X with X being a type, and it was unexpected that it actually referred to a new member ident because I've never seen a member declared in type's header

Maybe we don't need the ident to be variable – we could have a standard ident that's available in abstract contexts, something like thisAbstract

If onsite getters land then the original code snippet would be simpler: var self(() -> (cast this: MyAbstract), never):MyAbstract;, but I still see the value in making this a standard feature

@RealyUniqueName
Copy link
Member

RealyUniqueName commented Sep 11, 2019

I associate as X with X being a type

What about in?

abstract MyAbstr(MyClasss) in <ident>
@RealyUniqueName
Copy link
Member

RealyUniqueName commented Sep 11, 2019

Also let's do the same thing for the underlying type:

abstract MyAbstr(MyClass in <identUnderlying>) in <identAbstract> {

because similar trick is also used a lot if an abstract doesn't have to UnderlyingType but accepts other instances of that abstract as methods arguments:

abstract Money(Int in int) {
    @:op(A + B) @:commutative static function add(a:Money, b:Money):Money {
        return new Money(a.int + b.int);
    }

    public function new(amount:Int) {
        this = amount;
    }
}
@Gama11
Copy link
Member

Gama11 commented Sep 11, 2019

I don't see how using in helps. It doesn't distuingish it from types because there's already the quasi-deprecated import <Type> in <Alias>; syntax. I think in both cases it sounds more awkward than as.

@kevinresol
Copy link

Can we just use the abstract keyword?

@ncannasse
Copy link
Member

I'm okay with the feature but it all depends on the syntax. I would not like another magic identifier so it requires some kind of declaration. Maybe something like

@:this var self(get,never) : MyAbstract;

@nanjizal
Copy link

I think my use case is related...
I would like to use Array Access internally within an abstract when abstracting an Array, Haxe seems fine with this[0] but not with [0], the second I am told refers to self which is not supported?
The problem abstract code ..

public var red( get, set ): Float;
    function get_red(): Float {
        return [ 0 ];
        //return read( 0 );
    }
    function set_red( v: Float ): Float {
        [ 0 ] = v;
        //write( 0, v );
        return v;
    }

For example and more context, this experiment works, but I had to call methods because within the abstract it did not seem possible to use array access:
http://try-haxe.mrcdk.com/#d6192
Instead it was suggested over the haxe discord channel that I use self by Gama11:
http://try-haxe.mrcdk.com/#e71a1
( rem to switch haxe ver in compiler options ).
But if you notice in the generated Javascript code - there seems more overhead with a self getter/setter than in my original code, self is not optimum unless the compiler does some magic to reduce it back to my original read and write methods.

From a users perspective it would seem logical that an abstract should allow me to clarify between self and this, I really don't like some obscure @ macro stuff.

I am pretty sure if I understand Nicolas, his proposal will not be something I will forget and I will spend half an hour every time I need to use it trying to work out which subsection of the abstract documentation covers it's use or which pull request references it. I think it is far more natural to have a magic 'self'.... - it's needed for any advanced abstract use, further it should not ( ideally ) add a function call overhead as a getter setter seems to - lots of compiler magic is ideal here!!

From a language design perspective abstracts are great they allow you to structure your data more like a 'Data Orientated Design' so for example I was exploring storing my WebGL positions and colours in Arrays but then using abstracts to allow easier and cleaner access and creation.

I think 'self' as a keyword should be added since abstracts need them, self seems pretty clear term. Also I would still like to be able to iterate over abstract enums without having to google for a macro :)

anyway just my feelings on self perhaps they will provide different perspective.

@Simn
Copy link
Member

Simn commented Jun 19, 2020

We didn't reach a consensus on this one in our haxe-evolution meeting yesterday.

We agree that this feature is useful, but the syntax remains an open question.

@nanjizal
Copy link

nanjizal commented Jun 19, 2020

I think generally abstracts suffer from a lot of 'this' compared to classes could we just use "_" to reduce noise, I think it would be really easy to remember.

_[0]
this[0]

For any cases where _ is not clear where it may conflict with other _ use the user can just define a local var however they want.

var here =_;
var abstract =_;
var self =_;

and if it has a parent abstract you can access that via

var parentAbstract = _.super;
var grandParentAbstract = _.super.super;

Well just adding ideas as really keen on an implemention, so maybe it will sporn other ideas.

@nanjizal
Copy link

nanjizal commented Jun 19, 2020

fair enough if your not keen on that suggestion, but I don't want self problem to be ignored.

how about allowing the user to define the keyword, but they have to define it if they need self, this is not too hard to remember and it similar style to forward.

@:self('myInt') 
@:forward
abstract MyInt( Int ) from Int to Int {

Technically this is useful because you could access 'myInt' from within a child inherited, but would need to error if an inherited abstract tried to reuse the same self.

@:self('mySpecial') 
@:forward
abstract MyIntSpecial( MyInt )
@nadako
Copy link
Member

nadako commented Jun 20, 2020

Let's not overload _ with even more meanings please :)

@markknol
Copy link
Member Author

@nanjizal You can always do abstract MyAbstract(MyType) as _ if you really want to use that 😉 That's the nice thing, you can name it however you want. However if you want super in abstract, that would require another proposal I guess. But I personally didn't have a need for that so far, because I don't have such deep layers of abstractiness.

@kevinresol
Copy link

Just wanna bump the idea of using the abstract keyword itself.

abstract is now/potentially used as:

  • type declaration (abstract Foo(String))
  • type modifier (abstract class)
  • field modifier (abstract function)

I think there will be no problem if we use it in expression-level for self-access? This won't break anything because even in macro it would just be EConst(CIdent('abstract')), and it requires no addition syntax to declare (e.g. as/in/@:this as proposed by others).

What is the exact reason to reject this syntax proposal?

@RealyUniqueName
Copy link
Member

We also want something for accessing this on an abstract instance. I guess if we go for abstract as proposed by Kevin, then we can use .this.
I just don't like the keywords to be overloaded with even more different meanings.

@grepsuzette
Copy link

grepsuzette commented Oct 14, 2020

@Aurel300 I think this suffers the same problem as Simn proposal above.
The self doesn't refer to the underlying type since it is already referenced by this.

Therefore abstract Foo(anyIdent:Int) {} feels wrong to me because it seems to associate anyIdent with Int, while the discussion is about associating it to Foo.


Personally I see no big problem with the original proposal,

abstract MyAbstract(MyType) as <ident> {}

it can be misread by a newbie (confusion with import __ as __), but a newbie will have to read the doc when he stumbles upon abstract anyway.

And:

  • damn easy to remember
  • damn fast to use and convenient!

(I didn't know the trick detailed in the rendered version, underusing the abstract feature because of that, so I'm very much in favor of some simple solution).

@grepsuzette
Copy link

Alternatively, since abstracts requiring a self access are far from being the majority, how about a meta like @:abstractSelf(self) abstract Foo (String) {}?

This is self-documenting and awareness of that feature can be maximized by a simple paragraph in the documentation.

@kevinresol
Copy link

kevinresol commented Oct 16, 2020

abstracts requiring a self access are far from being the majority

Just to clarify that self access is not uncommon, I use it a lot for toString() and alike.

and wanna raise again the abstract keyword (with a snippet this time), which is not used in expression level right now and will involve no breaking change to macro API (just add CIdent('abstract'))

I just don't like the keywords to be overloaded with even more different meanings.

abstract is a noun in this context anyway (referring to the "abstract" type), so I think this self-access usage is not overloading its meaning, just reusing it.

abstract Error(Int) {
	var Forbidden = 403;
	var NotFound = 404;
	// ...
	
	public function toString() {
		switch abstract {
			case Forbidden: 'Forbidden';
			case NotFound: 'Not Found';
		}
	}
	
	public function foo(bar:Int) {
		abstract.bar(bar); // to resolve shadowed field
	}
	
	public function bar(v:Int) {}
}
@grepsuzette
Copy link

Ah suddenly I understand it with your example.

Find the abstract is an elegant solution, but it may be worth posting more examples e.g. with operator overloading because it falls into place so neatly all the question should revolve around the aesthetics with this solution: is it shocking or not.

@grepsuzette
Copy link

grepsuzette commented Oct 16, 2020

@kevinresol
Ok I edit my message, my other arguments were pretty weak:

But. who's to say that one day, we won't have a use for var e = abstract { some anon } or any expression with abstract in front of an anon?
If we use abstract as a special variable, we won't have it anymore as a keyword. I think this is a potential problem. I know it is far fetched (I looked too much C/C++ code maybe), but I think it is wiser to keep abstract as a reserved keyword than as a special variable name.

@nanjizal
Copy link

nanjizal commented Oct 23, 2021

Another time I come accross this awkwardness, and there is still no decision, it's definitely a hole in Haxe typesystem that needs plugging.

https://try.haxe.org/#34fE48A9

just to clarify when I tried to switch on self compiler complained and suggested I need to cover '_' which I couldn't add or else how is the compiler helping. So I need 'self' of some form. The ( this: MyAbstract ) is really hard to remember and ugly to use for such a common situations.

I think 'thisSelf' or 'thisAbstract', although both quite long, will be easy to understand for a new user.

Likely I am being nieve but can the compiler not inject the required code when 'thisAbstract' is referenced, perhaps if a user need to allow dynamic/reflection access then some @keepThisAbstract or something, or perhaps 'thisAbstract' is always added but DCE will remove if not required.

  // implementing self to make it easier to switch.
  // ideal if compiler added this and DCE unused ones away.
  public var thisAbstract( get, never ): MyAbstract;
  public inline function get_thisAbstract(): MyAbstract {
    return ( this: MyAbstract );
  }

Seems very simple approach, it may not cover all bases but it would certainly go a long way.

@nanjizal
Copy link

The simple injection would be fairly quick and easy to implement? And could be improved later if it was needed.

@Simn
Copy link
Member

Simn commented Oct 23, 2021

If you read the previous comments you will find that the problem isn't the implementation but the syntax.

@nanjizal
Copy link

Well I was taking Kevin's post forward and trying to get some traction that some decision is needed, and 'abstract' itself is nice but problematic.

'thisAbstract' seems clearer than 'abstract', in that there will be less user confusion.

So sometimes users will want
this

other times they will want
thisAbstract

seems really plain simple and any users of abstract types will understand that 'this' is the internal implementation and thisAbstract is ( this, MyAbstract ). I guess one other alternative...

thisMyAbstract

but that feels a bit too magic.

There are other options I see above suggested like 'in' and more messy '@' stuff, but to me they are overly complex for average and common use case, we would spend our time googling to remember the syntax. The situation seems very simple and outlined in the very first post by markknol, we just need 'self' and just picking a good name is needed.

@nanjizal
Copy link

I vote anyway that next time compiler team meet there should be a decision, it's a type hole for several years now, so bumping thread a little :) But thanks anyway Simn your doing great job, even if some of us like me are annoying.

@haxiomic
Copy link
Member

haxiomic commented Oct 23, 2021

thisAbstract has the benefit that it can be discovered by users rather than learned through the manual; users start typing this, which if they don't know abstracts well they won't necessarily know if it refers to the abstract or the underlying type, but they will also see thisAbstract suggested, allowing them to quickly figure out which one of the two they need for their tasks

@hughsando
Copy link
Member

I think the proposed solution is not too bad:

abstract Foo(asInt:Int) {

Where specifying an alternative for "this" frees up "this" to be the Foo. So kind of oldway/new way with no breaking change.

@lublak
Copy link

lublak commented Oct 24, 2021

What about this.this

@kLabz
Copy link

kLabz commented Oct 24, 2021

I think the proposed solution is not too bad:

abstract Foo(asInt:Int) {

Where specifying an alternative for "this" frees up "this" to be the Foo. So kind of oldway/new way with no breaking change.

That's the syntax I'd want but having this referring to underlying type or the abstract itself depending on abstract declaration might be confusing. But still is my preferred alternative.

@markknol
Copy link
Member Author

While giving this another thought. A breaking change (but probably a good/correct one) would be super to access the underlaying type and this access the self type.

@Simn
Copy link
Member

Simn commented Oct 25, 2021

While giving this another thought. A breaking change (but probably a good/correct one) would be super to access the underlaying type and this access the self type.

Frankly that's my preference at this point as well. It's pretty silly because it's clearly backwards, but maybe I can find some mental gymnastics to justify it...

@Simn
Copy link
Member

Simn commented Oct 25, 2021

Er, sorry, I read that backwards too. Of course I don't want to break it, but just use super to access the abstracts.

@kLabz
Copy link

kLabz commented Oct 25, 2021

Er, sorry, I read that backwards too. Of course I don't want to break it, but just use super to access the abstracts.

Doesn't make much sense though =/ I get the point about no breaking change, and it's an important one, but having this reference the abstract and super the underlying type, when used from within the abstract, is the one that makes sense, not the other way around. @hughsando's proposal at least goes in this direction; it's not perfect but I see it as more future proof.

@kevinresol
Copy link

While giving this another thought. A breaking change (but probably a good/correct one) would be super to access the underlaying type and this access the self type.

While this is more "semantically correct", it could cause silent breaking change:

abstract Foo({x:Int}) from {x:Int} to {x:Int} {
  var x(get, never):Int;
  function get_x() return 1;
  public function print() {
    return this.x;
  }
}
({x:0}:Foo).print(); // before change: 0; after change: 1 => breaks silently
@Simn
Copy link
Member

Simn commented Oct 25, 2021

That one seems really impractical. It would mean that you end up replacing all this whenever you want to access the abstract, which is even more wonky.

@nanjizal
Copy link

well with

abstract Moo( p: Int )
abstract Foo( p: Moo )

you could reference the Int with

super.super

But it's horrible to break so much existing code.

struggling to see how super could be made to work, found a rather ugly approach...

So for current this ( Moo ) there would be two options ( old ) and ( new ):

this  OR  this.super

and for Foo the abstract one.

this.abstract

then you could write from within Foo to access Int :

this.super.super

So many users could continue to use 'this' but if they wanted more control they could use 'this.super', and 'this.abstract'.

@TheDrawingCoder-Gamer
Copy link

i really like the abstract idea. why not reuse a keyword we already have? And it makes more sense imo. You access this abstract with abstract. About your child abstracts, that's tricky. I don't know what you should use. Maybe this.abstract ? And for the super super idk. this is just silly at this point. this.super.super could work but I'm unsure I like that syntax.

@Simn Simn added this to the 2021-12 milestone Nov 8, 2021
@Aurel300
Copy link
Member

Aurel300 commented Nov 9, 2021

Let's have a summary of all the ideas so far… For an abstract type Abs defined over T (abstract Abs(T)):

Declared as Local access to Abs Local access to T Instance access to x:Abs Instance access to x:T Breaking change Source
abstract Abs(T) as ident ident this x - N original proposal
- self this x - N original proposal, alternatives
- thisAbstract this x - N comment
abstract Abs(T) in ident ident this x - N comment
abstract Abs(T in identT) in identAbs identAbs this (or identT) x x.identT N comment
- abstract this x x.this N comment, comment
@:this var ident(get, never):MyAbstract ident this x - N comment
- _ this x - Y comment
@:self("ident") abstract Abs(T) ident this x - N comment
- virtual, unreal, notional, here, that, hereof this x - Y comment
abstract Abs(identT: T) - this (or identT) x x.identT N comment
@:abstractSelf(ident) abstract Abs(T) ident this x - N comment
- thisAbs this x - Y comment
abstract Abs(identT:T) this identT x x.identT Y comment
- this.this this x - N comment
- super this x - N comment
@Simn
Copy link
Member

Simn commented Nov 15, 2021

This proposal has been accepted, see https://haxe.org/blog/evolution-meeting-2021/ for details.

@Simn Simn merged commit b8379f2 into HaxeFoundation:master Nov 15, 2021
@Simn Simn added accepted and removed lean-accept labels Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment