Skip to content

feat: handle empty-body functions in smir.json. #753

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 21, 2025
Merged

feat: handle empty-body functions in smir.json. #753

merged 9 commits into from
Oct 21, 2025

Conversation

Stevengre
Copy link
Contributor

@Stevengre Stevengre commented Oct 18, 2025 •
edited
Loading

  1. Handle empty-body functions for linking and proving;
  2. Log empty-body function existence when logging the reduced items table size;
  3. Quick-compact for Statics, VTable and Function encoding.

@Stevengre Stevengre requested review from JuanCoRo, dkcumming, jberthold and tothtamas28 and removed request for jberthold October 20, 2025 03:34
@Stevengre Stevengre marked this pull request as ready for review October 20, 2025 03:35
@Stevengre Stevengre mentioned this pull request Oct 20, 2025
Open
3 tasks
@dkcumming
Copy link
Collaborator

Is this a duplicate of #677 ?

@jberthold
Copy link
Member

Is this a duplicate of #677 ?

I think it does the minimum necessary to avoid crashing on these things in input data (allocs) but does not model the data in any way.
OK to merge for now but maybe we should merge the other PR instead to avoid duplicating work.

jberthold
jberthold reviewed Oct 20, 2025
View reviewed changes
kmir/src/kmir/alloc.py Outdated
Comment on lines 9 to 12
if TYPE_CHECKING:
from typing import Any
# No conditional typing-only imports needed currently
pass
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this can be deleted then?

@Stevengre
Copy link
Contributor Author

Is this a duplicate of #677 ?

If this PR merged, I can modify this one to avoid duplicity.

@Stevengre Stevengre marked this pull request as draft October 20, 2025 13:03
@Stevengre Stevengre self-assigned this Oct 20, 2025
@Stevengre Stevengre changed the title feat(alloc): quick-compat for Static, Function and VTable feat: handle empty-body functions in smir.json. Oct 20, 2025
@Stevengre
Copy link
Contributor Author

By logging the empty-body functions after reducing, we know that these functions wouldn't effect the result of the current spl-token verification (at least with current spec).

@Stevengre Stevengre marked this pull request as ready for review October 20, 2025 14:18
jberthold
jberthold approved these changes Oct 20, 2025
View reviewed changes
kmir/src/kmir/kmir.py Outdated
has_missing = len(missing_body_syms) > 0
_LOGGER.info(
f'Reduced items table size {len(smir_info.items)}; '
f'missing-bodies-present={has_missing} count={len(missing_body_syms)}'
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should not output this unless has_missing is true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, changed it!

@automergerpr-permission-manager automergerpr-permission-manager bot merged commit 9c279b8 into master Oct 21, 2025
7 checks passed
@automergerpr-permission-manager automergerpr-permission-manager bot deleted the jh/quick-alloc branch October 21, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jberthold jberthold jberthold approved these changes

@tothtamas28 tothtamas28 Awaiting requested review from tothtamas28

@JuanCoRo JuanCoRo Awaiting requested review from JuanCoRo

@dkcumming dkcumming Awaiting requested review from dkcumming

Assignees

@Stevengre Stevengre

Labels

automerge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Stevengre @dkcumming @jberthold