Skip to content

Disallow exact references in public types #7554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: with-basic-inexact
Choose a base branch
from

Conversation

tlively
Copy link
Member

@tlively tlively commented Apr 25, 2025

When custom descriptors are disabled, validate that public types do not
contain exact references. If they did, we would drop the exactness and
change the identity of the public type during binary writing, which
would be incorrect. This still allows internal usage of exact types
without custom descriptors enabled, and it is up to the individual
passes to ensure that the eventual erasing of exactness does not cause
any problems.

When custom descriptors are disabled, validate that public types do not
contain exact references. If they did, we would drop the exactness and
change the identity of the public type during binary writing, which
would be incorrect. This still allows internal usage of exact types
without custom descriptors enabled, and it is up to the individual
passes to ensure that the eventual erasing of exactness does not cause
any problems.
@tlively tlively requested a review from kripken April 25, 2025 21:22
// during binary writing and change the public type identities.
if (module.features.hasCustomDescriptors()) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

if (module.features.hasCustomDescriptors()) {
return;
}
for (auto type : ModuleUtils::getPublicHeapTypes(module)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this adds a scan of the entire module in the common case (no CD, and validation is enabled)?

Perhaps we can avoid that overhead by instead erroring in the binary writer? That code already scans the module for heap types, and we could reuse that scan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants