wabt
wabt copied to clipboard
WebAssembly
replace old style union in initializing expression representation with std::variant
Hi, this is just small cleanup.
I've replaced union with std::variant to free Type type because now it should be always POD as a member of union.
Suddenly, after refactoring I revealed that FuncRef and NullRef cases and Type actually not used in our code so I don't move them into new version of InitExpr.
Are there any remaining issues with landing this? It would be helpful for completing the implementation of typed funcrefs and also GC support, as it would allow including a Var or similar data in the type.
Are there any remaining issues with landing this? It would be helpful for completing the implementation of typed funcrefs and also GC support, as it would allow including a
Varor similar data in the type.
It looks like this actually has to be rebased to be mergeable now, since the relevant file has been updated recently. I can work on rebasing this
I've rebased this now on this branch https://github.com/takikawa/wabt/commit/904827773db8764f27c6a356bcc7699fffe864ae
@dbezhetskov could you update your PR branch with this commit and re-push? Thanks.
This seems like a smart change for the reasons @takikawa mentioned. I personally don't really like using
std::visitandoverloadedvery much since it is a little complex, and would prefer using a switch/index instead. But it's mostly just a personal opinion, and I see that usingoverloadeddoes provide better type safety.
I think I agree. I prefer the old switch style. Especially if that allows use to avoid adding all those new struct types.
In the interests of trying to clean up the PR backlog -- is there still interest in pursuing this (subject to comments) or is this ok to close?
Hi @keithw, thanks for checking in. I personally don't have the bandwidth to pursue this currently and am ok with closing it. WDYT @dbezhetskov?