Treat untyped element in tuple and named tuple

[makenowjust]

Fixed #6718

The reason of #6718 is calling node.update with untyped TupleLiteral or NamedTupleLiteral. An untyped node has no node.program, so it fails nil assertion.

I think this node.update is not needed because CleanupTransformer does not change the type of node except for untyped and unreachable node. However such nodes should be transformed to other node. In other words node.update does nothing in general.

Thus, this replaces node.update with treating untyped element.

[asterite]

I don't think this is OK. What if some expressions inside the tuple are not untyped? Then, super should take care of handling untyped expressions. And then I can't see untyped expressions in the tests. I might give this a try and fix it.

[makenowjust]

Yes, there is no untyped expression in tests at least on cleanup phase after super called. It's important thing. (and I noted this as comment in spec.)

Maybe it is the simplest solution is moving node.update in behind of NoReturn processing and adding node.program = @program in front of node.update.
Perhaps it works fine. But, I'm wondering why this node.update is really needed.

[makenowjust]

And, the above I commented is very conservative way.

[asterite]

I don't understand, why is there a "Nil assertion failed"? Because @program is nil?

[straight-shoota]

Because @program is nil?

Yes, looks like it.

[makenowjust]

Yes, @program is nil. It is not assigned a value to node.program (node is TupleLiteral or NamedTupleLiteral). This assignment is done in MainVisitor usually. However, in the test case, the block of pending is not processed by MainVisitor due to not invoked (pending has no yield), so the tuple literal inside block is untyped and its node.program is nil.

[asterite]

I see.

Well, we have two solutions:

1. Assign @program before calling node.update
2. Remove @program altogether. It's needed to create a TupleType (program.tuple_type(...)) but we all Types have a reference to the Program, so we can simply do: program = elements.first.type inside TupleLiteral#update (after the first return unless...). Same goes for NamedTupleLiteral)

I much prefer the second option. It makes the code more robust (we don't need to remember to assign @program) and it makes TupleLiteral occupy less memory.

@makenowjust Would you like to do 2, or do you want me to do that?

[makenowjust]

@asterite Removing node.program is good idea, but I don't know it works for an empty tuple and empty named tuple...

And it seems you are sure node.update is needed. But I'm unbelieving in it. Could you explain about this?

[asterite]

@makenowjust Ah, right, I forgot about the empty case. Then yeah, let's set node.program = ... before calling node.update.

[straight-shoota]

🏓

[straight-shoota]

I'm a bit lost on how this should proceed... Who needs to take what action?

[makenowjust]

Updated.

[RX14]

FYI the correct english is "... it is needed to have an expression that is ..." and it reads even better as "... it is required to have an expression that is ..."

[straight-shoota]

Thanks @makenowjust !