tl icon indicating copy to clipboard operation
tl copied to clipboard

Record embed and const field syntax

Open pigpigyyy opened this issue 4 years ago • 10 comments

Implement the "embed" and "const / readonly" field functions for OOP simulations to support some Lua project uses. The "embed" function is refactored from @euclidianAce's implementation ( mentioned in issue #97 ) with bug fixes.

pigpigyyy avatar Sep 06 '21 07:09 pigpigyyy

Teal Playground URL: https://474--teal-playground.netlify.app

github-actions[bot] avatar Sep 06 '21 07:09 github-actions[bot]

Hi @pigpigyyy, thank you for taking the time to make a PR!

The code itself looks good, but I'm uncertain about adding these features to the language. These are the reasons:

  • The embed feature seems to suggest a structural subtyping system, but Teal records are nominal. I'm thinking of adding explicit subtyping, but that needs cleaning up the logic for covariant/contravariant checks — since Lua tables are used in both modes, there would need to be some annotation for covariant/contravariant arguments, and making this in a non-confusing manner for using is an interesting design problem.

  • The readonly feature for record fields is interesting, but I'm afraid it might provide a false sense of security, since fields could be rewritten by simply aliasing the record to a different type...

I'd like to hear what you and other users think about these concerns!

hishamhm avatar Sep 09 '21 18:09 hishamhm

The readonly feature for record fields is interesting, but I'm afraid it might provide a false sense of security, since fields could be rewritten by simply aliasing the record to a different type...

In most languages there are also ways to get around fields being public, private and/or protected. Yet those languages don't give that idea up and properly setting the access for fields is encouraged instead of having everything be public.

Also, by that same logic, the entire type system gives a false sense of security as a simple as can be used to give a table to a function that takes an integer.

I think it is best to treat as as how Rust treats Transmute, it is there if you really need/want it but... YOU need to make sure you don't screw up.

lenscas avatar Sep 26 '21 14:09 lenscas

I'm using Teal in a C++ project with Lua embedded, and get a lot of native OOP stuff exported into Lua. So I just seek for a way to do the simulated OOP check. Since C++ does not support covariant/contravariant features by default, I'm not worrying too much for it. As for nominal records, I just changed the way to compare records with embeds in order to make subtypes to be not equal.

pigpigyyy avatar Sep 27 '21 08:09 pigpigyyy

I think it is best to treat as as how Rust treats Transmute, it is there if you really need/want it but... YOU need to make sure you don't screw up.

@lenscas What I meant by aliasing is different from an explicit as because there are places where we have bivariant behavior (due to lack of proper support for record subtyping yet) so that causes implicit aliasing. I agree that in explicit as, all bets are off. My concern was about implicit aliasing and changing those rules in the future, hope that clarifies!

hishamhm avatar Sep 27 '21 16:09 hishamhm

As for nominal records, I just changed the way to compare records with embeds in order to make subtypes to be not equal.

Thanks @pigpigyyy, I'll give it another review!

hishamhm avatar Sep 27 '21 16:09 hishamhm

I think it is best to treat as as how Rust treats Transmute, it is there if you really need/want it but... YOU need to make sure you don't screw up.

@lenscas What I meant by aliasing is different from an explicit as because there are places where we have bivariant behavior (due to lack of proper support for record subtyping yet) so that causes implicit aliasing. I agree that in explicit as, all bets are off. My concern was about implicit aliasing and changing those rules in the future, hope that clarifies!

Ah, yes then I fully agree to wait until that to see if readonly is a good fit.

lenscas avatar Sep 27 '21 18:09 lenscas

@hishamhm Did you get a chance to look into this a bit more? I'm personally loving the "embed" design and implementation here. It's feels almost like intersection types from TypeScript.

I'm not worried about type-soundness, but being able to express another much-common usage of tables in the type system. I'm considering taking this PR in a temporary fork if you're likely get this or a something similar PR in the future. (I'm currently using TypescriptToLua but taking a serious look at Teal.)

// ETA: Some idle discussion about syntax aside. Not directly related to the PR. BIG changes proposed as thought experiments //

  • Consider swapping syntax {string:number} with {[string]number} (":" doesn't make sense in the former anyway.)
  • Tuples and arrays stay the same {integer}; {number, boolean}
  • Consider anonymous record type: {a: integer, b: number} (more intuitive IMHO)
  • Consider swapping syntax function(integer,number):number with function(integer,number)->number, (":" doesn't make sense..)
    • (Or even just (integer,number)->number...)
  • Implement intersection types (or something like it)
    • "extend"/"embed" syntax can just be "&" types
    • Array records: {integer} & {a:integer, b:number}
    • Userdata: Userdata & {x: number, t: boolean} (Or something similar... )
  • Enums can be "|" types. local type SwitchStatus = "on" | "off"
  • Perhaps keep the current syntax "deprecated" for a while
  • A parser/prettyprinter combo can keep help write "converter" tools to update "deprecated" syntax

Not sure if syntax discussions are closed for good or not, but these are some thoughts coming from TypeScript (specifically using it to compile Lua) pov. HTH.

// ETA 2 (Can you tell I'm passionate about this? 😄) Some more realistic (but probably not enough) syntax ideas around types. //

local type Pos = record {x:number, y:number}
local type Size = record {w:number, h:number}
local type Color  = record {r:number, g:number, b:number, a:number}
-- example with "embedded" types, similar to intersection types of TS
-- this maybe easier to implement then `Pos & Size & ....` style syntax, which may require a new `typealias` directive.
local type MyItem = record(Pos, Size, Color, {max_x: number, maX_y: number})
local type MyOption = enum("on","off")

// ETA 3 -- Updated opinions after exploring the ecosystem //

While I still like the syntax I suggested in previous edits, I think it would be unrealistic since there seems to be a big enough userbase to break. Not sure if they're implementable without breaking existing code so I'm changing my thinking on this. I think the syntax implemented in this PR seems fine.

I now recommend that

  • readonly/const parts are removed
  • rest is cleaned a bit (there's a few small issues)

If @hishamhm accepts this, I'd be happy to the a full review of this PR, and or update it if @pigpigyyy is not interested in this anymore.

Thanks.

muratg avatar May 27 '22 04:05 muratg

@muratg Thanks for the feedback!

Regarding this feature proposal, I think the direction Teal should go is to adopt real intersection types. Various features in the language already hint in this direction (arrayrecords, polymorphic function members in records) so it would be cleaner to replace them with intersection types. I'm sure that intersections would end up having some limitations similar to those we currently have with unions, but I think overall it would make the design cleaner (and the language hopefully smaller, which is not a hard goal but always a hint of elegance).

Regarding the other syntax changes, yeah, no intention on changing them now. (Side comment: it's always interesting to see people's different perceptions... most of your suggestions lean towards making it look more like Typescript or Go, which is probably what you're most used to! "Intuitive" is always relative to the beholder — Teal's syntax style follows on the Lua tradition, which is in itself more aligned to the Pascal/Modula family than that of curly-bracket-style languages).

hishamhm avatar May 31 '22 16:05 hishamhm

I agree familiarity is important in deciding what's intuitive. My main issue with colon in map and function syntax was more about : being overused. In the typing context, a colon usually comes after a variable name or a function argument. But for map it's coming after another type, for example. There's a similar issue in Typescript too. But in Lua/Teal syntax it's a slightly bigger issue, because : is also used to denote "methods", both in definition and use site.

But I can totally see your point of view too. It makes sense to target existing Lua users for Teal instead of other languages.

Looking forward to seeing the intersection types! I think it should have less limitations compared to union types since it's an extension process and not a contraction. If you already thought of its syntax, please share!

muratg avatar May 31 '22 16:05 muratg

I'm closing this PR since we'll be getting functionality to address similar use-cases via the interface type as already implemented in the next branch! Thank you all for the brainstorming and feedback!

Interfaces still need some work (generics, ensuring the subtyping relations all work as intended (tests!!!!)) but I'm pretty happy with the implementation to the point that I've already ported the compiler's own type objects to use them :)

https://github.com/teal-language/tl/tree/next

hishamhm avatar Jan 08 '24 22:01 hishamhm

Just started testing the Next Teal! Everything is supposed to be there except the "readonly field" feature, I will try making a new PR with the newest version. Thanks for your effort!

pigpigyyy avatar Jan 09 '24 03:01 pigpigyyy