HaxeFoundation / haxe-evolution

Repository for maintaining proposal for changes to the Haxe programming language
111 stars 58 forks source link

`@:native` for structure fields #32

Closed nadako closed 6 years ago

nadako commented 6 years ago

Rendered version


typedef JsonSchema = {
  @:native("enum") var enum_:Array<JsonValue>;
}

schema.enum_ = [1,2,3]; // generates schema.enum = [1,2,3]
print(schema.enum_); // generates print(schema.enum)

See https://github.com/HaxeFoundation/haxe/issues/5105 and https://github.com/HaxeFoundation/haxe/pull/2185

nadako commented 6 years ago

I can work on implementing this, but I need some review and approval of the idea by @ncannasse and/or @Simn first.

ncannasse commented 6 years ago

I think an easier version would be to allow { "enum" : 5 } and have it typed as { "enum" : Int } (both would be allowed in terms of Haxe syntax).

In that case it might still be escaped as _enum or $enum or something else if enum is a keyword in the generated platform.

nadako commented 6 years ago

I think an easier version would be to allow { "enum" : 5 } and have it typed as { "enum" : Int } (both would be allowed in terms of Haxe syntax).

That would also require some field access syntax like schema."enum", but yes that would also do the job. Althought I thought you were against exactly this in https://github.com/HaxeFoundation/haxe/pull/2185 and asked for the @:native proposal.

What should we do?

ncannasse commented 6 years ago

Uhm right, I didn't took field access into account, and we don't want to allow either value."enum" or value["enum"]

On Thu, Oct 5, 2017 at 11:10 AM, Dan Korostelev notifications@github.com wrote:

I think an easier version would be to allow { "enum" : 5 } and have it typed as { "enum" : Int } (both would be allowed in terms of Haxe syntax).

That would also require some field access syntax like schema."enum", but yes that would also do the job. Althought I thought you were against exactly this in HaxeFoundation/haxe#2185 https://github.com/HaxeFoundation/haxe/pull/2185 and asked for the @:native proposal.

What should we do?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HaxeFoundation/haxe-evolution/pull/32#issuecomment-334406625, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-bwC6fntHsUG07GeneBtRCiwTgoEb5ks5spJzwgaJpZM4PuQSV .

nadako commented 6 years ago

we don't want to allow either value."enum" or value["enum"]

While I can see why we don't want the general value["enum"] syntax (since it looks like a map lookup), I'm not sure why you're against value."enum" - it's clearly a field access and doesn't suggest any map-like behaviour. I consider this kind of identifier escaping.

Btw, I think C# has value.@enum syntax for this, which could be an option too (tho maybe can be a bit confusing wrt our metadata syntax).

nadako commented 6 years ago

Anyway, I'm waiting for directions on what to do next: proceed with the proposed @:native stuff or implement the quoted fields syntax.

clemos commented 6 years ago

While I think it would be nice to be able to use value.enum directly (I have no idea but maybe it would be possible to differentiate field names and keywords ? In JS, using such keywords as field names is perfectly valid), I think it should be done in another feature request.

ncannasse commented 6 years ago

@Simn what do you think about us allowing <expr>.<keyword> ?

Simn commented 6 years ago

Well, let's talk about consistency here:

That doesn't strike me as good design. Allowing expr."keyword" would at least be consistent with the structure values, and also at least similar to the type field version because "at least they both use string literals".

kevinresol commented 6 years ago

I am bringing this up again: https://github.com/HaxeFoundation/haxe-evolution/issues/22.

I suppose in some other languages there are concepts of contextual keywords (e.g. await in C#), that some identifiers are only considered as keywords in particular contexts.

IMO, we could implement that step by step. As starter, field.keyword should be allowed as there is no ambiguity. (though how to define the field is another topic)

clemos commented 6 years ago

@Simn Ultimately, I think it would make sense to allow { keyword : value } too (once again, like JS does). Fields on types certainly bring more issues, but I think we could consider allowing them too (could only be accessed via this.keyword or Class.keyword though) Anyway, I agree with @kevinresol : #22 is probably worth reopening.

nadako commented 6 years ago

can we go with @:native solution that works for now and then you guys discuss new syntax?:)

ncannasse commented 6 years ago

I fail to see how that would even work, for instance with this:

typedef Args = { var x : Int; @:native("new") var y : Int; };

class Test {
    static function foo() return { x : 0, y : 0 };
    static function main() {
         var v : Args = foo();
    }
}

If it's just to allow assigning literals I think it's not good enough.

kevinresol commented 6 years ago

The fun fact is that on js you can use keywords as object field:

var o = {new:1}; console.log(o.new); this runs fine in js

clemos commented 6 years ago

@ncannasse well @:native already has this problem https://try.haxe.org/#36B22 so this wouldn't really be a con for this feature :S Obviously, I think everybody agrees allowing keywords as field names would be cleaner...

nadako commented 6 years ago

Seriously, people? I covered this exact case in the proposal...

kevinresol commented 6 years ago

Seriously, people? I covered this exact case in the proposal...

Sorry I misunderstood Nicolas's comment. Yeah, the example case makes sense.

nadako commented 6 years ago

And no, I think allowing keywords is not a solution, because you still can't use fields like not-valid in @clemos example. So It's either the quoted field syntax or @:native, the latter being easier to implement and not introducing new syntax.

kevinresol commented 6 years ago

I think the keyword stuff is a bit off-topic here, sorry for bring that out at first place. (but I still hope it could be re-considered) Personally I think OP is a good solution with least effort in current situation.

ncannasse commented 6 years ago

Ok, I forgot that you have dealt with it in the proposal, sorry.

Reading again the whole thing, while I'm still not 100% satisfied with the solution, I don't have either a better proposal to make at this point, so feel free to move forward with the change.

kLabz commented 6 years ago

Any news on this? Would be much appreciated when dealing with externs for react libs :)

hamaluik commented 6 years ago

I also need this for dealing with externs for various projects, most lately: Blender externs.

nadako commented 6 years ago

I have a branch somewhere... Will try to find, revive it and get into Haxe 4

Simn commented 6 years ago

It bothers me that this affects unification... It shouldn't be necessary to have typing jump through hoops because the syntax is insufficient to express what you want to express. It's not even strictly related: The shortcoming here is in the field-access syntax, and our solution is to meddle with the struct declaration and its unification behavior.

I don't know if we're still voting, but this one would get a "meh" from me. I acknowledge the problem, but I don't think this is the best solution.

nadako commented 6 years ago

Well, but the problem is severe and the fix like this, even temporary would be appreciated, also I don't think the change in the code base will be that intrusive. Anyway, suggestions are always welcome :)

Simn commented 6 years ago

I still think field."name" is the obvious solution here. It solves exactly what we want to solve and whatever problems it brings at generation-level would also be brought by @:native.

hamaluik commented 6 years ago

I don't know anything about the compiler or unification, but I can add from a user's perspective the original proposal matches exactly what I expect to happen based on how @:native works in other extern places (and I was a bit surprised at first when I naively tried to apply @:native on a field name and it didn't work like it does elsewhere). I get that "working the same as elsewhere" might not be strictly correct from a compiler / language standpoint, but I think it meshes with the intention of the @:native metadata (to transform Haxe types into native types, which would be expanded into: transforming Haxe types and fields into native types and fields).

I think the field."name" syntax is a bit confusing—it suggests to me that I could do something like field.'name${5*25}', or even var s:String = "abc"; field.s = true; (what would it export as? field.s or field.abc? Or throw an error?)

markknol commented 6 years ago

I'm in favor of allowing @:native on typedefs. It would help on the my hxobfuscator project, which shortens/obfuscates field names by decorating fields with @:native.

kLabz commented 6 years ago

It seems like we will need some sort of solution to this problem in the near future for haxe js (well, react): React will most probably drop className in favor of class in the next major version (discussion here) (+ for instead of htmlFor).

I'm not even sure @:native would be enough for this use case, since when doing jsx('<div class="myclass">hello world</div>') there isn't any typedef for the props atm. Well, this one needs a macro anyway so it could be handled somehow.

Simn commented 6 years ago

or even var s:String = "abc"; field.s = true; (what would it export as? field.s or field.abc? Or throw an error?)

How would field.s ever resolve to field.abc or throw an error? It's the normal field syntax and totally unaffected by anything.

Anyway, it feels like I'm on a different planet than everybody else here. So if you want to go ahead with @:native I'm not gonna stop you.

kLabz commented 6 years ago

expr."keyword", while not being pretty (imo), may be the cleanest (and more robust) solution indeed.

It also solves the field."not-valid" case @clemos and @nadako mentioned, which may come up in the future (for js target at least).

I think the field."name" syntax is a bit confusing—it suggests to me that I could do something like field.'name${5*25}'

We'd just need an error stating that single quotes are not allowed for quoting field names. Or allow string interpolation here..?

As for @:native use on typedefs, I think we need a compilation warning (which can be disabled?) telling us that it only works on types (with a hint to the quoted alternative). I'm not sure it is clear enough atm that it won't work on typedefs, and it just fails silently (until a runtime error occurs).

kevinresol commented 6 years ago

Or allow string interpolation here..?

You can't interpolate in field names in ObjectDecl already {'$x':1}, so I think it is ok.

And it is quite consistent as well. I mean field."keyword" and {"keyword": value}

kLabz commented 6 years ago

Yep :+1:

What about typing this with typedefs? Would it be typedef A = { "keyword":String } / typedef A = { var "keyword":String; }?

nadako commented 6 years ago

I'm gonna close this proposal as it's clear that we don't have consensus. As I mentioned before, I think obj."field" is a nice(r) option as well, I just thought it was rejected before. Anyway, this will require another design proposal that someone has to make.

kLabz commented 6 years ago

@ncannasse

we don't want to allow either value."enum" or value["enum"]

Do you have a strong opinion against value."enum"? If so, could you elaborate a little?

farteryhr commented 5 years ago

Yep 👍

What about typing this with typedefs? Would it be typedef A = { "keyword":String } / typedef A = { var "keyword":String; }?

just thinking more on induction: var "keyword": String; (ok in typedef. what about in a class? what about in a block?) "keyword": String (ok in typedef, what about in an argument list?)

and more idea (mostly a general identifier quote):

oops

i think i found something good? any cons please?

examples:

typedef JsonSchema = {
  var \enum:Array<JsonValue>;
}

schema.\enum = [1,2,3];
print(schema.\enum);

var \var = 1;
const { \class, \for } = prop;

// maybe allow omitting \ on "xxx" when possible?
someHaxeDiv.style.\"background-color" = rgb(234,130,32); // just demo
{\"background-color": rgb(234, 130, 32)}
nadako commented 4 years ago

FWIW I implemented field."name" syntax here https://github.com/HaxeFoundation/haxe/pull/9433. Can't say I'm a huge fan of this, but we need some solution.

markknol commented 4 years ago

So this issue can only be tackled at the call side and not at the definition side?

nadako commented 4 years ago

So this issue can only be tackled at the call side and not at the definition side?

With @:native solution, it's just the definition. With field-quoting solution it's both definition and access.

markknol commented 4 years ago

Ah that sounds great!

sonygod commented 3 years ago

I can't wait for this.