reflect: fully implement Type.AssignableTo #4376
Conversation
This is a big reimplementation that simplifies the compiler a lot. Instead of storing the method set in metadata and lowering the type asserts as a whole program pass, this change just puts the list of methods in the type code (and a separate global for the interface type).
This fully implements AssignableTo, fixing a number of bugs in the previous implementation. In particular, it now supports AssignableTo for interface types.
| @@ -65,7 +65,6 @@ func Optimize(mod llvm.Module, config *compileopts.Config) []error { | |||
|
|
|||
| // Run TinyGo-specific optimization passes. | |||
| OptimizeStringToBytes(mod) | |||
| OptimizeReflectImplements(mod) | |||
There was a problem hiding this comment.
So what happens with type asserts that are statically known to be true or false? Optimization pushed for future work?
There was a problem hiding this comment.
This is specifically for the reflect Implements method, which is used in some important code but not really used that much in total (so I don't think optimizing it will provide much benefit). The main reason this "optimization" existed was to get the encoding/json package to work.
But yeah, good point about other (normal Go style) interface type asserts. I can do a check how common that is and whether optimizing that case is beneficial. That is something that should work well as a package level optimization though.
|
@dgryski any progress on this? This is likely causing issues for me. |
This fully implements AssignableTo, fixing a number of bugs in the previous implementation. In particular, it now supports AssignableTo for interface types.
Depends on #4375