Skip to content

Conversation

@Stevengre
Copy link
Contributor

@Stevengre Stevengre self-assigned this Dec 15, 2025
@Stevengre
Copy link
Contributor Author

Stevengre commented Dec 15, 2025

  • test program for testing this.

Add a test case that reproduces issue #55 where function pointers
passed as arguments (e.g. to Option::map) are not captured in the
functions array.

Also update normalise-filter.jq to filter unstable type IDs in
ArrayType.size.kind.Value.
Zero-sized constants can represent function items (FnDef) used as values,
e.g. when passing a function pointer to a higher-order function.
This ensures such functions are included in the link map so they appear
in the `functions` array of the SMIR JSON.

Fixes #55
@Stevengre Stevengre force-pushed the jh/print-function-pointer branch from 787a38b to ea3606b Compare December 15, 2025 12:50
When LINK_INST is disabled, the link map key only contains Ty without
instance kind, so different instances (Item vs ReifyShim) of the same
function type would conflict.

Instead of using a heuristic preference function, we now:
- Skip ReifyShim entries if an Item entry already exists
- Replace ReifyShim entries with Item entries when encountered

This is cleaner because ReifyShim has no body in items (it's generated
by the linker), so Item is always more useful.
@Stevengre Stevengre force-pushed the jh/print-function-pointer branch from ea3606b to 940f2da Compare December 15, 2025 13:16
@Stevengre Stevengre force-pushed the jh/print-function-pointer branch from 31f5ec3 to 0253b9c Compare December 15, 2025 13:51
# Remove the hashes at the end of mangled names
.functions = ( [ .functions[] | if .[1].NormalSym then .[1].NormalSym = .[1].NormalSym[:-17] else . end ] )
| .items = ( [ .items[] | if .symbol_name then .symbol_name = .symbol_name[:-17] else . end ] )
| .items = ( [ .items[] | if .symbol_name then .symbol_name = .symbol_name[:-17] else . end ] | map(walk(if type == "object" then del(.ty) else . end)) )
Copy link
Member

@jberthold jberthold Dec 16, 2025

Choose a reason for hiding this comment

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

This is not necessary because
a) it is done again in line 6
b) there can only be NormalSym, NoopSym or IntrinsicSim, with no further fields.

Sorry, I misread this. However, we should place this further down in the "filtering" section instead of up here where we only want to adjust the function names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved

# delete unstable alloc, function, and type IDs
| .allocs = ( .allocs | map(del(.alloc_id)) | map(del(.ty)) )
| .functions = ( [ .functions[] ] | map(del(.[0])) )
| .functions = ( [ .functions[] ] | map(del(.[0])) | map(walk(if type == "object" then del(.ty) else . end)) )
Copy link
Member

@jberthold jberthold Dec 16, 2025

Choose a reason for hiding this comment

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

not necessary because there can only be NormalSym, NoopSym or IntrinsicSim, with no further fields.

Suggested change
| .functions = ( [ .functions[] ] | map(del(.[0])) | map(walk(if type == "object" then del(.ty) else . end)) )
| .functions = ( [ .functions[] ] | map(del(.[0])) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved.

@Stevengre Stevengre requested a review from jberthold December 16, 2025 02:04
Copy link
Member

@jberthold jberthold left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adjusting!

@automergerpr-permission-manager automergerpr-permission-manager bot merged commit 626e8fe into master Dec 16, 2025
5 checks passed
@automergerpr-permission-manager automergerpr-permission-manager bot deleted the jh/print-function-pointer branch December 16, 2025 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants