search

wren-lang / wren

The Wren Programming Language. Wren is a small, fast, class-based concurrent scripting language.
http://wren.io
MIT License
6.9k stars 552 forks source link

Did you notice that `_` and `__` are a valid field name in Wren? #792

Open ChayimFriedman2 opened 4 years ago

ChayimFriedman2 commented 4 years ago

I don't know whether it's a bug, but it seems like one...

The guilt is here:

https://github.com/wren-lang/wren/blob/039150efebdffcfe850813d13892d2619721df2c/src/vm/wren_compiler.c#L1049-L1052

We do not check that there is a letter or digit (or underscore), not here nor in readName():

https://github.com/wren-lang/wren/blob/039150efebdffcfe850813d13892d2619721df2c/src/vm/wren_compiler.c#L782-L803

And we're finishing up with an identifier in length 0 😄 Funny.

ruby0x1 commented 4 years ago

Nice find! It doesn't seem intentional.

mhermier commented 4 years ago

I knew it, and I agree it can be miss interpreted because the doc might say it introduce special identifiers, while even the '_' is part of the identifier so it is not 0 length. So AFAIK it is not really a bug, but an ambiguity of the documentation and/or implementation detail.

ChayimFriedman2 commented 4 years ago

Oh right, not zero. I was mistaken because of the consumption of the first _.

mhermier commented 4 years ago

Unless there is plan to forbid them, the issue can be closed.

ChayimFriedman2 commented 4 years ago

Are you sure this isn't a bug? If this is by design I'll close the issue, but I suspect that only Bob can answer this... @munificent ?

munificent commented 4 years ago

It definitely wasn't intentional. I'll let @ruby0x1 decide if this is a bug or a, uh, bonus feature. Personally, I would lean towards making it an error, but I don't have a strong preference.

mhermier commented 4 years ago

About pro and cons of letting, for me the biggest pros is that other language accept those names and it doesn't introduce an arbitrary limitation that need to be checked.

About the cons, it split the name of the variable, which can make it easier to teach maybe?

But my biggest issue about this, and I remember discussing about it with @munificent in some form when it was designed, we talked about a way to distinguish between variables types. @munificent opted for a neat hack that implied selection by pattern matching, but I also exposed the possibility to unary operators. And the issue with what is proposed is that it tries to make '_' and '__' some kind of operator like thing.

I mean, if operator style is the way to go, lets drop '_' and '__' and use a dedicated set of not over-loadable unary operators like '*', '**' or what ever and the limitation proposed will naturally emerge from that decision.