mojo icon indicating copy to clipboard operation
mojo copied to clipboard

[BUG]: Trailing comma is not supported in assignemnts

Open kishmakov opened this issue 1 year ago • 17 comments

Bug description

Python supports trailing comma for multiple targets in assignment expressions, while Mojo doesn't. It contradicts the goal to become a Python superset.

Steps to reproduce

Here is a snippet which demonstrates the issue.

def main():
    var a: Int = 0
    var b: Int = 0
    a, b = 1, 2 # works
    a, b, = 1, 2 # fails
    print("a is", a)
    print("b is", b)

System information

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 22.04.2 LTS
Release:	22.04
Codename:	jammy
$ mojo --version
mojo 0.6.3 (f58f8b94)
$ modular -v
modular 0.2.2 (95d42445)

kishmakov avatar Jan 22 '24 11:01 kishmakov

I am against supporting trailing comma. It leads to strange bugs if sth like this would be used:

x = 1,  # comma is accidental and thus type of x is tuple[int] instead of int

IMHO Mojo should not support Python weird quirks which more often lead to strange bugs than to increased usability. Mojo cannot be compiled Python and sacrificing compatibility in such cases seems to be a step in good direction

gryznar avatar Jan 22 '24 11:01 gryznar

By the way, this issue (x = 1,) looks rather like a tooling issue to me, not like a language issue.

With proper tooling support, such accidental bugs can be marked with diagnostic hints.

kishmakov avatar Jan 22 '24 14:01 kishmakov

So, why Mojo would have to support this controversial syntax if tooling would highlight that?

gryznar avatar Jan 22 '24 14:01 gryznar

@gryznar I think you better address this question either to Modular team (superset approach) or to Python team (initial language design).

Every choice obviously has its pros and cons. And there is some motivation behind it.

kishmakov avatar Jan 22 '24 14:01 kishmakov

@kishmakov Yeah, my all remarks are addressed directly to Modular team to not introduce this "missing feature"

Not all Python behavior is possible to replicate in compiled language, so, not all features can be applicated directly. Mojo cannot be strict superset, so it has to compromise. If so, compromising here to avoid strange bugs seems like not a bug and that is what I would to say. Bare comma at the end, without parentheses is not what I ever have to use in Python, same as most users, so really, this looks for me like a good decision, that Mojo does not support that

Personally, Mojo will be the best if it would be a Python++ and not emulated Python. I agree that all these incompatible decision should be published in FAQ with answer to "Why ... is not supported?"

gryznar avatar Jan 22 '24 14:01 gryznar

Thanks for opening this ticket! We discussed the problem of trailing commas in unpacking as well as in function call expressions (e.g. foo(a, b,), which is supported in Python). We decided that this is something we want to support; perhaps it looks quirky, but it is part of Python, and syntactic compatibility with Python is an important goal of Mojo. We acknowledge that in some cases syntactic compatibility might be at odds with building a better Python, but we think this particular feature poses no such risks. This means, that we will treat this ticket as a genuine bug, and will try to fix it.

laszlokindrat avatar Feb 13 '24 20:02 laszlokindrat

But, the question is: what is the added value from this support? As I stated it is not useful in most cases and may lead to very strange errors. I eencountered such one and it was waste of time to track this because Python automatically converted trailing comma to unwanted tuple. Really, this is the latest feature which I would Mojo to support. I propose to make a pool to community for such quirks and ask if it is desired or not.

You can always add it, but it's harder to remove later.

gryznar avatar Feb 13 '24 20:02 gryznar

@laszlokindrat Can we at least have the LSP generating warnings/fixes like "redundant trailing comma" etc.? Also, will code compatibility be that much of a problem? Mechanical upgrader is needed (wrapping certain names in backquotes, e.g.) any ways.

soraros avatar Feb 13 '24 20:02 soraros

@laszlokindrat as you can see, there are voices from community which do not want this feature. Please, do not ignore them. Pylint treats trailing comma like a possible bug: https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/trailing-comma-tuple.html. If there are hundreds of bugs in Mojo to fix, do we really, really want to focus on adding support for such buggy feature? If linters highlight that as possible bug, why to support it at all?

gryznar avatar Feb 13 '24 20:02 gryznar

@gryznar @soraros I agree that there are issues which are much more pressing than this. Our decision above was with respect to what we would like to achieve in the fullness of time: syntactic compatibility with Python. Notably, we did not decide to prioritize this: it might happen soon if some other related feature gets prioritized and the fix becomes easy, or it might be a while before we have a fix.

But, the question is: what is the added value from this support?

One of the reasons why trailing commas are something we need to support is they provide a way to control formatting in a Pythonic way. For example, adding a trailing comma will trigger black to separate the expressions to multiple lines, and potentially add a pair of parentheses as well, e.g. a, b, = 1, 2, becomes

(
    a,
    b,
) = (
    1,
    2,
)

Python users do rely on this feature, and it is reasonable to assume that Mojo users would too.

Pylint treats trailing comma like a possible bug

This seems like a reasonable thing to do in a linter, where users can also opt out of it. But that doesn't mean that language should disallow it.

laszlokindrat avatar Feb 13 '24 21:02 laszlokindrat

I am still not convinced. How about opening a discussion / poll and asking community itself? Maybe there is no such demand at all?

gryznar avatar Feb 13 '24 21:02 gryznar

@laszlokindrat please at least reject tuple creation or more generally - signature change using trailing comma. This will prevent accidental mistakes like:

x = 1, #  type of rhs is tuple instead of int

The rest of usecases related to formatting only which does not impact type or signature, may be indeed usable

gryznar avatar Feb 13 '24 21:02 gryznar

@gryznar Of course we would love to hear more from the community! As you pointed out though, this issue is not super high priority so, at this time, I don't see a point driving a bigger discussion around this. Feel free to leave comments on this ticket; we are following all these threads and we are keeping an open mind.

laszlokindrat avatar Feb 13 '24 22:02 laszlokindrat

I think the trailing comma and other syntactic peculiarities should be removed because Mojo already will not be a strict superset of Python. Python already occupies the ^ operator for a bit operation, and Mojo's use of it as a transfer directly conflicts with it's use in Python, so Mojo is not a strict superset of Python unless the transfer operator is removed or renamed, which seems highly unlikely. Since that means Mojo will be a loose superset, it seems reasonable for a loose superset to avoid Python's pitfalls.

LiamSwayne avatar Mar 04 '24 20:03 LiamSwayne

I think it's also worth noting that ~, |, >>, << and & also could not be repurposed if Mojo plans on being a strict superset. These are Python features that very few Python developers use, but occupy quite a few symbols that could have other uses.

LiamSwayne avatar Mar 04 '24 20:03 LiamSwayne

@LiamSwayne The binary ^ in Mojo is still the same bit operation as in Python, and I don't see why it's in conflict; if using the same symbol for two things is desirable is a separate issue.

soraros avatar Mar 04 '24 23:03 soraros

This is a simple parser omission. Adding it improves python compatibility and comes at no cost.

lattner avatar Apr 09 '24 02:04 lattner