Conversation
scripts/test/shared.py
Outdated
| 'imports.wast', # Requires fixing handling of mutation to imported globals | ||
| 'proposals/threads/imports.wast', # Missing memory type validation on instantiation | ||
| 'linking.wast', # Missing global type validation on instantiation | ||
| 'linking.wast', # Incorrectly allows subtyping for table imports |
There was a problem hiding this comment.
Does this mean the test is wrong or that our implementation is wrong?
There was a problem hiding this comment.
Our implementation is wrong, table types should be invariant for imports but I implemented them as covariant (same mistake as Java): link. I'll address this in the next PR.
| bool isSubType(const Global& global) const; | ||
|
|
||
| private: | ||
| const Global definition; |
There was a problem hiding this comment.
Can we have this be a pointer to the underlying definition owned by some defining module? It's a little weird to have a Global that is just hanging out and not owned by some module.
There was a problem hiding this comment.
I think that would make the lifetimes/pointers more complicated. Now we have
ModuleRunner -> RuntimeGlobal
If we made the global a pointer, it would be
ModuleRunner -> RuntimeGlobal -> ModuleRunner -> Module
Basically the RuntimeGlobal would have to point back at the Module owned by ModuleRunner. In practice everything will live as long as the ModuleRunner, but it would be hard to tell from reading the code and if more complicated cases some up.
I also think that it's ok for a Global to stand on its own. It might be instantiated in different modules, so the Global itself isn't tied to a particular instance, only the RuntimeGlobal is. Does that seem reasonable?
2591c89 to
ba2c167
Compare
ba2c167 to
6b63ad6
Compare
Example error message:
Part of #8261.