move icon indicating copy to clipboard operation
move copied to clipboard

[move-compiler] Support casting without parentheses

Open macalinao opened this issue 3 years ago • 4 comments

Motivation

Easier to read and type if casts do not require enclosing parentheses.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Updated language tests, and ran:

cargo test --workspace --exclude move-prover --exclude move-to-yul --exclude evm-exec-utils -- --skip prove

macalinao avatar Sep 01 '22 19:09 macalinao

I'm not sure what the consequences are. There is likely a reason why there are braces (like certain syntax constructs become ambiguous otherwise), but perhaps also not. Do you remember @tnowacki ?

Unfortunately, these small changes look nice and harmless isolated but may have more indirect impact than we understand.

wrwg avatar Sep 01 '22 21:09 wrwg

I'm not sure what the consequences are. There is likely a reason why there are braces (like certain syntax constructs become ambiguous otherwise), but perhaps also not. Do you remember @tnowacki ?

Unfortunately, these small changes look nice and harmless isolated but may have more indirect impact than we understand.

Is there a way we can figure out what the impact could be via test cases? If so, what are some test cases we can construct to test this?

macalinao avatar Sep 02 '22 00:09 macalinao

I'm not sure what the consequences are. There is likely a reason why there are braces (like certain syntax constructs become ambiguous otherwise), but perhaps also not. Do you remember @tnowacki ? Unfortunately, these small changes look nice and harmless isolated but may have more indirect impact than we understand.

Is there a way we can figure out what the impact could be via test cases? If so, what are some test cases we can construct to test this?

We need @tnowacki input here because he was present when this grammar was designed.

wrwg avatar Sep 03 '22 04:09 wrwg

Agreed that we need @tnowacki's input here; I don't have enough context on the grammar. I do have some ideas for precedence stress tests:

  • my_num / 2 as u64
  • my_num as u64 as u64
  • a as u64 / b as u64

sblackshear avatar Sep 03 '22 15:09 sblackshear

This PR has been orphan for a while. Everybody seems to want it but concerns are around breaking changes. @macalinao are you intending to continue work on this? Then please feel free to re-open. @tnowacki do you want to take this issue on your todo list? Then we may want to turn this into an issue instead.

wrwg avatar Oct 17 '22 02:10 wrwg

Filed an issue so at least this work won't be lost

tnowacki avatar Oct 17 '22 18:10 tnowacki

This PR has been orphan for a while. Everybody seems to want it but concerns are around breaking changes. @macalinao are you intending to continue work on this? Then please feel free to re-open. @tnowacki do you want to take this issue on your todo list? Then we may want to turn this into an issue instead.

I spoke with @macalinao a few weeks back about closing another PR he had open. Turns out he's very busy right now so we can probably carry on the work.

oxade avatar Oct 17 '22 18:10 oxade