Open
Conversation
325b0fd to
48a5757
Compare
… the constructor from the Slang object
48a5757 to
d27a570
Compare
1a70afb to
bc3b4af
Compare
bc3b4af to
bc1f2cf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this one started as a though experiment based on the repetitive pattern of checking an instance in order to create the correct wrapping object.
👎 We lose some typechecks as this solution is more general.
😐 The types for the constructors and the keys in the
Mapcould use some work.😐
YulStatementandStatementhad to check manually forYulBlockandBlockbecause it generated a cyclical dependency (I could have solved this by having the factory be called in a different file but that was a bit too much and the noise is not that big).👍 Built size decreased by 5KB 😮
Update:
Instead of a
forloop we can use aMapusing the Slang class constructor as key for aMapand use that tofindthe corresponding AST constructor. We would lose the proper inheritance tree check thatinstanceofdoes. (slight performance increase in exchange for theinstanceofmatch)Also if we choose to use the class name as the key, we can use an
objectinstead of aMapwhich would result in a performance improvement, but that would deviate from the currentinstanceofapproach.I'm open to feedback here. (slightly bigger performance increase in exchange for a much looser match)