wasm-merge: check that the types of imports and exports match. #6437
Conversation
vouillon
force-pushed
the
merge-typing
branch
4 times, most recently
from
60a2fc4 to
65b6a70
Compare
src/tools/wasm-merge.cpp
Outdated
| void reportTypeMismatch(bool& valid, const char* kind, Importable* import) { | ||
| valid = false; | ||
| std::cerr << "Type mismatch when importing " << kind << " " << import->base | ||
| << " from module " << import->module << ": "; |
There was a problem hiding this comment.
Can also print the import->name here. That would differentiate imports with the same module/base but different internal names, which happens in the test (which is good to test, that's an important corner case).
| << " from module " << import->module << ": "; | |
| << " from module " << import->module << "(" << import->name << "): "; |
Please add internal names to the test, so that this information becomes more useful (rather than the parser autogenerating internal names).
There was a problem hiding this comment.
The internal name is usually not very informative. One often reuses the same name as the base:
(import "env" "foo" (func $foo ...)
So, I'm concerned that the import could get quite often renamed, and thus that printing this new name might actually be confusing. The best thing to do would be to print the initial name as well as the name of the file containing this import, but we no longer have this information.
There was a problem hiding this comment.
I agree the original file would be useful, yeah. Too bad we don't have it.
I still think the internal name can help. How informative it is depends on the toolchain, but regardless, when there are duplicate imports it is really the only way to differentiate anything. I do suggest (in the diff above) to put the internal name in parentheses because I agree it is less informative than the other information.
|
I fuzzed this for a while and saw no issues. |
No description provided.