Skip to content

Variant constructor experiment#1413

Open
Janther wants to merge 17 commits intomainfrom
constructor-experiment
Open

Variant constructor experiment#1413
Janther wants to merge 17 commits intomainfrom
constructor-experiment

Conversation

@Janther
Copy link
Member

@Janther Janther commented Feb 12, 2026

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 Map could use some work.
😐 YulStatement and Statement had to check manually for YulBlock and Block because 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 for loop we can use a Map using the Slang class constructor as key for a Map and use that to find the corresponding AST constructor. We would lose the proper inheritance tree check that instanceof does. (slight performance increase in exchange for the instanceof match)

Also if we choose to use the class name as the key, we can use an object instead of a Map which would result in a performance improvement, but that would deviate from the current instanceof approach.
I'm open to feedback here. (slightly bigger performance increase in exchange for a much looser match)

@Janther Janther requested a review from fvictorio February 12, 2026 14:32
@Janther Janther force-pushed the constructor-experiment branch from 325b0fd to 48a5757 Compare February 12, 2026 16:37
@Janther Janther force-pushed the constructor-experiment branch from 48a5757 to d27a570 Compare February 12, 2026 16:51
@Janther Janther force-pushed the constructor-experiment branch from 1a70afb to bc3b4af Compare February 18, 2026 02:00
@Janther Janther force-pushed the constructor-experiment branch from bc3b4af to bc1f2cf Compare February 18, 2026 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments