Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug report: wrong error message in integer division #926

Closed
vbuterin opened this issue Jun 23, 2018 · 7 comments
Closed

Bug report: wrong error message in integer division #926

vbuterin opened this issue Jun 23, 2018 · 7 comments
Labels
bug Bug that shouldn't change language semantics when fixed.

Comments

@vbuterin
Copy link
Contributor

> from vyper import compiler
> compiler.compile("@public\ndef foo():\n  z:int128 = 5 / 2")
vyper.exceptions.TypeMismatchException: line 3: Invalid type, expected: int128
  z:int128 = 5 / 2

The "expected" type should be decimal, not int128.

@ben-kaufman
Copy link
Contributor

I think the message is correct.
The variable z is an int128 so it expects to receive int128, while 5 / 2 is a decimal. So basically you did try to give an int128 variable a decimal value so the compiler says it expected to receive an int128.

Sent with GitHawk

@fubuloubu
Copy link
Member

I thought we changed int/int to return an int. Sounds like there's problems with constants in this regard.

@ben-kaufman
Copy link
Contributor

I think we agreed to change that but it looks like it doesn't work.
Looks like the bug is that int/int returns a decimal instead of an int.

@ben-kaufman
Copy link
Contributor

ben-kaufman commented Jun 24, 2018

I checked that out and it looks like the compiler automatically checks if the result is an int.
So if you try the same but with 4 / 2 it will compile to int128 but when the division results in a decimal number it will stay as a decimal (which is good imo).

@fubuloubu
Copy link
Member

Nice. Stick a convert in there is you want to avoid there error!

@jacqueswww jacqueswww added the bug Bug that shouldn't change language semantics when fixed. label Jun 25, 2018
@jacqueswww
Copy link
Contributor

jacqueswww commented Jun 25, 2018

@vbuterin @fubuloubu @ben-kaufman I am not completely sure what we agreed upon with constants/literals case.
The current implementation takes a literal and simplifies it to a single constant ✅
But it might be considered inconsistent with the behaviour of using integer division when using variables. ❔
The related accepted VIP is as follows:

#775

decimal [op] decimal = decimal (decimal division)
int128 [op] int128 = int128 (signed integer division)
uint256 [op] uint256 = uint256 (unsigned integer division)
decimal [op] integer = COMPILE TIME ERROR
integer [op] decimal = COMPILE TIME ERROR

If we want to keep literals consistent with the above agreed upon VIP.
We should do integer division and return an int128 (if it's in bounds of int 'course)?

@jacqueswww
Copy link
Contributor

jacqueswww commented Jun 25, 2018

^ thumbs up the above comment if you think we should alter to do [literal int] / [literal int] = int128 (using integer division) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug that shouldn't change language semantics when fixed.
Projects
None yet
Development

No branches or pull requests

4 participants