DeadArgumentElimination: Do not make private types public #7941
Conversation
| (type $A (struct)) | ||
|
|
||
| ;; CHECK: (func $export (type $3) (result (ref $A)) | ||
| ;; CHECK: (func $export (type $func) (result anyref) |
There was a problem hiding this comment.
Looks like this test needs updating (or at least the comment below does).
| ;; CHECK: (func $export2 (type $3) (result (ref $A)) | ||
| ;; CHECK-NEXT: (unreachable) | ||
| ;; CHECK-NEXT: ) | ||
| (func $export2 (export "export2") (result (ref $A)) |
There was a problem hiding this comment.
Another option would be to just put $A in the same rec group as $func.
| if (isExported) { | ||
| if (!privateTypes) { | ||
| privateTypes.emplace(); | ||
| for (auto type : ModuleUtils::getPrivateHeapTypes(*module)) { |
There was a problem hiding this comment.
It's actually much cheaper to just get the public types, since that only has to look at imports and exports.
|
Ah, I realized this entire optimization isn't valid. The fuzzer pointed out that the importing module can do an exact cast. That is, when custom descriptors are enabled, it isn't ok to refine an exported function: calling it is fine, but casting a reference of it to an exact type will notice the difference. (Which is kind of but not exactly a violation of substitutionality.) As the main idea was to use this to fuzz CD, disabling it in that mode seems sad 😄 Am I missing something @tlively or should I revert the last PR #7935? |
|
Casting the function type would be a violation of the closed-world contract, so we can probably keep doing this with Maybe we need a fuzzer mode that generates modules that take imports from another module, but also doesn't do anything that we assume never happens if the first module is a closed world. Reverting the previous PR makes sense to me :/ |
|
Yeah, we need some kind of a new fuzzer mode for this I guess... Harder than I thought. |
Followup to #7935