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

Sharp edges of the "ternary" operator #1907

Closed
dumblob opened this issue Sep 9, 2019 · 12 comments
Closed

Sharp edges of the "ternary" operator #1907

dumblob opened this issue Sep 9, 2019 · 12 comments
Labels
Bug This tag is applied to issues which reports bugs.

Comments

@dumblob
Copy link
Contributor

dumblob commented Sep 9, 2019

V version:
V 0.1.18 (git commit * 7fc678c 2019-09-09 fix urllib_test.v (HEAD -> master, origin/master, origin/HEAD) [Alexander Medvednikov]|)

OS:
Linux t500 4.19.67-1-lts #1 SMP Fri Aug 16 10:30:45 CEST 2019 x86_64 GNU/Linux

What did you do?
Neither of the following works. Each produces a very different error and this error is either fully misleading or a syntax generation bug in V compiler. Some of them may be related to #1108 though V compiler didn't produce any such indicative error.

import rand

fn f() ?int {
  rand.seed( 999 )
  return if rand.next( 9 ) <= 5 { panic( 'ERR <=5' ) } else { 3 }
}

fn main() {
  x := if f() <= 5 { 6 } else { 7 }
  println( x )
}
import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) > 5 { 3 } else { 4 }
  return x
}

fn main() {
  x := if f() <= 5 or { 5 } { 6 } else { 7 }
  println( x )
}
import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) <= 5 { panic( 'ERR <=5' ) } else { 3 }
  return x
}

fn main() {
  x := if f() <= 5 { 6 } else { 7 }
  println( x )
}
import rand

fn f() ?int {
  rand.seed( 999 )
  return if rand.next( 9 ) > 5 { 3 } else { 4 }
}

fn main() {
  x := if f() <= 5 { 6 } else { 7 }
  println( x )
}
import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) > 5 { 3 } else { 4 }
  return x
}

fn main() {
  x := if f() <= 5 { 6 } else { 7 }
  println( x )
}
import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) > 5 { 3 } else { 4 }
  return x
}

fn main() {
  println( if f() <= 5 { 6 } else { 7 } )
}
import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) > 5 { 3 }
  panic( 'if without else branch' )
  return x
}

fn main() {
  println( if f() <= 5 { 6 } else { 7 } )
}

What did you expect to see?
All of them are IMHO a valid syntax in V. So all of them shall be correctly compiled.

What did you see instead?
Many different wild errors.


P.S. It would be nice to make a test out of each reported issue and making it a commit policy to run all these regression tests first before merging to master.

@dumblob dumblob added the Bug This tag is applied to issues which reports bugs. label Sep 9, 2019
@joe-conigliaro
Copy link
Member

Hi @dumblob, we don't actually have a ternary operator.
Just this an if statement which can be used in assignment etc. There are a couple of things going on here. In the first example for starters f() is supposed to return an optional but you are returning an int or panicking, it would need to return an error instead of panic ( I think that might not work with that syntax & return at the moment).

Also the syntax is wrong it should be:

a := if num > 5 { '>5' } else { '<5' }

@dumblob
Copy link
Contributor Author

dumblob commented Sep 10, 2019

we don't actually have a ternary operator.

Yep, that's why I put it in double quotes (though it compiles down currently to a plain ternary operator in C).

In the first example for starters f() is supposed to return an optional but you are returning an int or panicking, it would need to return an error instead of panic ( I think that might not work with that syntax & return at the moment).

Yep, I expected something like that - that's why I made a set of examples which currently feel right according to the scarce information about V syntax & semantics. Once the syntax & semantics settles, we'll need to soften these sharp edges and make them working in some way to avoid "wtf" moments when exploring the language with the strategy: "V allows terse readable code, let's write that way" (i.e. shortest possible still readable code - i.e. panicking immediately instead of additionally returning a dummy int as unreachable code - maybe make panic() context-aware and always return a corresponding type as the other brother-branch in which panic() was invoked?).

a := if num > 5 { '>5' } else { '<5' }

I'll correct it. It's though interesting, than V didn't complain about it and in one case at least compiled the code without else 😮 (but the C compiler afterwards complained about syntax issues...).

@teggotic
Copy link

import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) > 5 { 3 }
  panic( 'if without else branch' )
  return x
}

fn main() {
  println( if f() <= 5 { 6 } else { 7 } )
}

main.v:7:8: unreachable code because you are trying to return after panic. panic indicates that you are leaving current function scope so anything after it in the same block is treated as unreachable code.

@teggotic
Copy link

Also if expression is very unstable as for now. You can use match expression.

@avitkauskas
Copy link
Contributor

@Danil-Lapirow Are you deliberately using panic() here instead of error()? Your last example that compiles and runs should be:

import rand

fn f() ?int {
  rand.seed( 0 )
  if rand.next( 9 ) > 5 { return 3 } 
  return error( 'rand was not greater than 5' )
}

fn main() {
  res := f() or { panic(err) }
  println( if res <= 5 { 6 } else { 7 } )
}

But yes, I agree that there are many small nuances in V now that are not handled in compiler and then results in errors in C compiler. I think the reason for that is simplified parsing and code generation without the AST and a rigorous analysis of it. I understand that looked nice and easy for @medvednikov in the start and made things run quicker also, but it scares me when thinking about the reliability of V in the future. This kind of ad-hoc aproach to fighthing errors in compiler without proper rigorous tests to avoid regressions is the biggest risk from my point of view regarding the bright future of V.

@gslicer
Copy link

gslicer commented Sep 11, 2019

Could an explicit "else" help?

if rand.next( 9 ) > 5 {
     return 3
} 
else {
    return error( 'rand was not greater than 5' )
}

In manner to en--force--courage to write code the same way, I think an "if" without following "else" shall be not valid (as a "match" without an default case).

@avitkauskas
Copy link
Contributor

@gslicer No. There are many situations when if without the else makes perfect sense (when you need something done only when some condition is met, otherwise - nothing).

@gslicer
Copy link

gslicer commented Sep 11, 2019

@gslicer No. There are many situations when if without the else makes perfect sense (when you need something done only when some condition is met, otherwise - nothing).

This "nothing" is exactly expressed with an empty else {} clause - but OK, maybe it makes sense to have some optional syntax and not enforce consistency all over the place

@ntrel
Copy link
Contributor

ntrel commented Sep 18, 2019

Looks like the errors are caused by:

  • or being declaration syntax, not an expression. Making it an expression would be a new feature.
  • Option<T> is not comparable with T (it should be).

@chanbakjsd chanbakjsd added Status: Needs More Info The issue author is expected to provide more information. question labels Sep 28, 2019
@dumblob
Copy link
Contributor Author

dumblob commented Jan 30, 2020

@lutherwenxu what info is still needed? I also fail to find any questions here. This is a pure buggy behavior IMHO 😉.

@chanbakjsd chanbakjsd removed Status: Needs More Info The issue author is expected to provide more information. Type: Question labels Jan 30, 2020
@medvednikov
Copy link
Member

Fixed.

@dumblob
Copy link
Contributor Author

dumblob commented May 31, 2020

@medvednikov I might be missing something, but many cases still do not seem to work:

case 1

import rand

fn f() ?int {
  rand.seed( 999 )
  return if rand.next( 9 ) <= 5 { error( 'ERR <=5' ) } else { 3 }
}

fn main() {
  x := if f()? <= 5 { 6 } else { 7 }
  println( x )
}

yields:

my_ternary.v:5:10: error: mismatched types `Option` and `any_int` 
    3 | fn f() ?int {
    4 |   rand.seed( 999 )
    5 |   return if rand.next( 9 ) <= 5 { error( 'ERR <=5' ) } else { 3 }
      |          ~~
    6 | }

case 2

import rand

fn f() int {
  rand.seed( 999 )
  x := if rand.next( 9 ) > 5 { 3 } else { 4 }
  return x
}

fn main() {
  x := if f() or { 5 } <= 5 { 6 } else { 7 }
  println( x )
}

yields:

==================
/home/user/.cache/v/my_ternary.tmp.c: In function ‘main’:
/home/user/.cache/v/my_ternary.tmp.c:10005:18: error: invalid initializer
10005 | Option_int _t1 = f();
      |                  ^
/home/user/.cache/v/my_ternary.tmp.c:10009:22: error: expected ‘;’ before ‘}’ token
10009 |  *(int*) _t1.data = 5}
      |                      ^
      |                      ;
/home/user/.cache/v/my_ternary.tmp.c:10010:14: error: invalid operands to binary <= (have ‘Option_int’ and ‘int’)
10010 | int x = (_t1 <= 5 ? (
      |              ^~
/home/user/.cache/v/my_ternary.tmp.c: In function ‘vcalloc’:
...
==================
(Use `v -cg` to print the entire error message)

builder error: 
==================
C error. This should never happen.

If you were not working with C interop, please raise an issue on GitHub:

https://github.com/vlang/v/issues/new/choose

You can also use #help on Discord: https://discord.gg/vlang

case 3 (basically the same as case 1 but with temporary variable)

import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) <= 5 { error( 'ERR <=5' ) } else { 3 }
  return x
}

fn main() {
  x := if f()? <= 5 { 6 } else { 7 }
  println( x )
}

yields:

my_ternary.v:39:8: error: mismatched types `Option` and `any_int` 
   37 | fn f() ?int {
   38 |   rand.seed( 999 )
   39 |   x := if rand.next( 9 ) <= 5 { error( 'ERR <=5' ) } else { 3 }
      |        ~~
   40 |   return x
   41 | }

case 4 (function lies it returns an Optional but in fact it doesn't)

import rand

fn f() ?int {
  rand.seed( 999 )
  return if rand.next( 9 ) > 5 { 3 } else { 4 }
}

fn main() {
  x := if f()? <= 5 { 6 } else { 7 }
  println( x )
}

yields (but should produce a different error - namely that the function in fact doesn't return any Optional despite having the return type specified as such):

==================
/home/user/.cache/v/my_ternary.tmp.c: In function ‘main’:
/home/user/.cache/v/my_ternary.tmp.c:10009:14: error: invalid operands to binary <= (have ‘Option_int’ and ‘int’)
10009 | int x = (_t1 <= 5 ? (
      |              ^~
/home/user/.cache/v/my_ternary.tmp.c: In function ‘vcalloc’:
/home/user/.cache/v/my_ternary.tmp.c:4802:1: warning: control reaches end of non-void function [-Wreturn-type]
 4802 | }
      | ^
/home/user/.cache/v/my_ternary.tmp.c: In function ‘byte_is_white’:
/home/user/.cache/v/my_ternary.tmp.c:7663:1: warning: control reaches end of non-void function [-Wreturn-type]
 7663 | }
      | ^
...
==================
(Use `v -cg` to print the entire error message)

builder error: 
==================
C error. This should never happen.

If you were not working with C interop, please raise an issue on GitHub:

https://github.com/vlang/v/issues/new/choose

You can also use #help on Discord: https://discord.gg/vlang

case 5 (same as case 4, but with a temporary variable)

import rand

fn f() ?int {
  rand.seed( 999 )
  x := if rand.next( 9 ) > 5 { 3 } else { 4 }
  return x
}

fn main() {
  x := if f()? <= 5 { 6 } else { 7 }
  println( x )
}

yields:

==================
/home/user/.cache/v/my_ternary.tmp.c: In function ‘main’:
/home/user/.cache/v/my_ternary.tmp.c:10010:14: error: invalid operands to binary <= (have ‘Option_int’ and ‘int’)
10010 | int x = (_t1 <= 5 ? (
      |              ^~
/home/user/.cache/v/my_ternary.tmp.c: In function ‘vcalloc’:
/home/user/.cache/v/my_ternary.tmp.c:4802:1: warning: control reaches end of non-void function [-Wreturn-type]
 4802 | }
      | ^
/home/user/.cache/v/my_ternary.tmp.c: In function ‘byte_is_white’:
/home/user/.cache/v/my_ternary.tmp.c:7663:1: warning: control reaches end of non-void function [-Wreturn-type]
 7663 | }
      | ^
...
==================
(Use `v -cg` to print the entire error message)

builder error: 
==================
C error. This should never happen.

If you were not working with C interop, please raise an issue on GitHub:

https://github.com/vlang/v/issues/new/choose

You can also use #help on Discord: https://discord.gg/vlang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This tag is applied to issues which reports bugs.
Projects
None yet
Development

No branches or pull requests

8 participants