warp icon indicating copy to clipboard operation
warp copied to clipboard

Handle literals

Open piwonskp opened this issue 2 years ago • 2 comments

Type of literal was previously determined by typestring. That means any type could be assigned to Literal, eg. literal could have uint8 type. InferType class infers the type based on node properties and structure thus have a specific set of type nodes that may be returned given literal node as an argument.

To conform to infertype class we could:

  • Keep using typestring parser only for literals
  • Convert literals to constants
  • Make subclasses for literals (Uint8Literal etc.)
  • Have a WarpLiteral class with a kind attribute

piwonskp avatar Jan 16 '23 15:01 piwonskp

I personally don't like the first option because it would mean the existence of thousands of line of code only to support the old way. @piwonskp do you have any prefered one? Do you have some insight on what would be the pros and cons of each one?

rodrigo-pino avatar Jan 31 '23 13:01 rodrigo-pino

Honestly I don't have a strong opinion on that. I agree that typestring parser is not an option long term. I'd see that rather as a temporary solution to fix other problems. Now if you ask me about my preferred solution. Philosophically speaking the best option would be to subclass solc-typed-ast's Literal. Why? Because the library do not provide the features we need. So an extension to its capabilities seems to be rational. Using constants for me does not seem like a real solution since we loose control over the output - we no longer can create plain literals in the output cairo code. So I consider using constants as a workaround which is a little bit hackish. It limits our capabilities. However on the other hand I don't think we care whether we produce literals or constants. So there might be more practical reasons, eg. simplification of code due to converting literals once to constants and not having to deal with both types of nodes afterwards. So the real question is whether converting to constants would simplify our codebase? Or maybe there is a reason why we did not do that before?

piwonskp avatar Jan 31 '23 16:01 piwonskp