Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

Handle literals #849

Open
Tracked by #818 ...
piwonskp opened this issue Jan 16, 2023 · 2 comments
Open
Tracked by #818 ...

Handle literals #849

piwonskp opened this issue Jan 16, 2023 · 2 comments
Labels
needs discussion Discuss solution before start implementing it

Comments

@piwonskp
Copy link
Contributor

piwonskp commented Jan 16, 2023

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 piwonskp added this to the solc-typed-ast 11.0.2 milestone Jan 16, 2023
@piwonskp piwonskp added the needs discussion Discuss solution before start implementing it label Jan 30, 2023
@rodrigo-pino
Copy link
Contributor

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?

@piwonskp
Copy link
Contributor Author

piwonskp commented Jan 31, 2023

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs discussion Discuss solution before start implementing it
Projects
Status: Todo
Development

No branches or pull requests

2 participants