Parsing ambiguity remains unsolved

[yanliu18]

The existing implementation of the mir parser failed at resolving parsing ambiguities, for example, EnumConstruct vs BinaryOperation.

All the following mentioned branches tested on k v6 installed via kup.

At the master branch, the parser failed at parsing the arithmetic-simple.mir, though kmir run is successful due to disambiguity rules.

At the feature/basic-operations branch, with added CheckedBinaryOp implemented in the same fashion, the parsing and execution failed due to parsing ambiguity. In particular, the execution was stuck at an intermediate result (only appears on linux machine not reproducible on mac, I am confused here. It might be the caching problem that I have been struggling with) like

_2 = amb ( amb ( Add ( const 1_usize , const 1_usize , .OperandList ) , Add ( const 1_usize , const 1_usize ) ) , Add ( const 1_usize , const 1_usize ) ) 

At the refactor/builtin-identifiers branch, I have attempted to implement the built-in functions as tokens, eliminating disambiguity rules. It has no parsing failures except when the variable name clashes with built-in functions (#171 ). I tend to go with this option where it seems straightforward for me (And definitely, there will be more work todo to make the test happy). However, I am unsure if any of you @geo2a and @virgil-serbanuta have tried this option before.
Is there any reason you choose to implement the solution on the master branch instead of this one? Any comments?

[virgil-serbanuta]

I think that BinaryOp was defined after I moved to Elrond, so I may misunderstand some things, but it seems that using BinaryOpName to define BinaryOp is a better solution. I think I tried to do similar things in the part of the semantics that I wrote. If I am mistaken with the above, and I actually wrote BinaryOp, I may have just forgot to add a TODO to use specific tokens instead of Identifier.

[yanliu18]

@virgil-serbanuta Thanks for the feedback. I was on audits for quite some time when KMIR was developed. But I don't want to trace back. I guess the issue #90 you submitted a while ago expects the implementation on branch refactor/builtin-identifiers.

[geo2a]

That was me who drafted the implementation based on runtime disambiguation. I have to admit that it was the first time I had to deal with ambiguities in K and it did not cross my mind that there was a simpler solution! @yanliu18 please go ahead with the implementation in refactor/builtin-identifiers.

[yanliu18]

That was me who drafted the implementation based on runtime disambiguation. I have to admit that it was the first time I had to deal with ambiguities in K and it did not cross my mind that there was a simpler solution! @yanliu18 please go ahead with the implementation in refactor/builtin-identifiers.

Sounds cool. I'll stick to refactor/builtin-identifiers implementation then. It was an interesting try with the runtime disambiguation. It could be helpful in future.
Thanks guys.