Fix type checking of tuple types in return statements

[davesque]

- What I did

Fixed #1006 .

- How I did it

Updated code in Stmt.parse_return.

- How to verify it

Run the tests.

- Description for the changelog

An issue with type checking of tuple types in return statements, which caused type checking to erroneously pass in certain cases, was fixed.

- Cute Animal Picture

[davesque]

@jacqueswww @fubuloubu

There's currently an issue with this "fix" that causes a number of tests to fail (https://dpaste.de/Y6hV#L566).

These modifications cause bytes[n] types with different values for n to be considered incompatible. I noticed that type checking for basic types in return statements only considers whether or not the canonical text representations of types are equal (https://github.com/davesque/vyper/blob/return-type/vyper/parser/stmt.py#L487-L488). Should we also be using the same kind of equality check for tuple element types? One way this could end up causing an issue is if the tuple contains another tuple as an element.

[fubuloubu]

I really think a more expressive, class-based type system would help here. That's a larger fix, but this is starting to look hacky

[davesque]

@jacqueswww @fubuloubu

By the way, it seems a bit odd that the two "empty" internal unit values, None and {}, appear to be treated differently by BaseType.__repr__. Here's an example:

(vyper) Davids-MacBook-Pro ~/projects/ethereum/vyper
$ ipython
/Users/david/.pyenv/versions/3.6.5/lib/python3.6/site-packages/IPython/core/interactiveshell.py:763: UserWarning: Attempting to work in a virtualenv. If you encounter problems, please install IPython inside the virtualenv.
  warn("Attempting to work in a virtualenv. If you encounter problems, please "
Python 3.6.5 (default, May 31 2018, 13:33:50)
Type 'copyright', 'credits' or 'license' for more information
IPython 6.4.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from vyper.types import BaseType

In [2]: bt1 = BaseType('int128', unit={})

In [3]: bt2 = BaseType('int128', unit=None)

In [4]: '{}, {}'.format(bt1, bt2)
Out[4]: 'int128, int128(*)'

What's the asterisk supposed to represent?

[davesque]

@fubuloubu I agree actually. I don't really consider this change to be a proper fix. I'm mostly using this PR to try and understand the mechanics of the issue and to act as a comments thread.

[fubuloubu]

I started trying to prototype something in blocktract, but I got hung up on figuring out the types of statements and stuff.

I think taking a more functional approach, with type classes (which may host units) and other objects representing operators, statements, functions, and contracts themselves would allow a more holistic approach with our own custom AST (that converts to LLL), avoiding issues like this in the long term (and make Vyper easier to work with!)

[davesque]

@fubuloubu Any thoughts about the asterisk thing in the comment above?

[fubuloubu]

Thoughts? It just looks wrong to me lol

[jacqueswww]

@davesque At this stage, we have to account for bytes[n] types having exact matching, this is mostly because I am pretty sure padding will not work on the byte level if we allow the n <= m rule. OK I looked at those specific tests and the return code. I think just apply the n <= m rule check here ie. bytearrays with smaller maxlen are allowed.

The * in int int128(*) notation indicates the position, which if it's a * means it exists at compile currently - ie. it's a literal.

[davesque]

@jacqueswww I noticed there's a "positional" property on BaseType. But I suspect that's not related to what you're talking about because it doesn't factor into whether or not a "*" appears in a BaseType instance's representation.

Also, why would a value being a literal be related to its type information? Shouldn't it still be compatible with any other value of the same type whether the value is determined at run-time or compile-time?

[jacqueswww]

So basically it's not the job of the ast to know this information at this stage, one needs this information to be able to operate on it later on the traversal.

A literal, in memory or storage variable have different ways of being accessed if used in larger expressions so that's why everywhere you see the '.typ.typ == .typ.typ' in the code where base (string) gets compared.

[jacqueswww]

@davesque wait a minute checking the '*' on repr, isn't it just the unit type?

[jacqueswww]

Fixed in #1175, with new TupleLike type.