Skip to content

TypeScript: read vtable entries as uint16 #8435

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
bjornharrtell merged 3 commits into from
Nov 27, 2025
Merged

TypeScript: read vtable entries as uint16 #8435

bjornharrtell merged 3 commits into from
Nov 27, 2025

Conversation

@jtbandes
Copy link
Contributor

@jtbandes jtbandes commented Oct 30, 2024 •
edited
Loading

According to the internals docs, vtable entries are uint16 rather than int16.

This change allows reading to work when vtable entries are larger than 2^15-1 (which already works in C++). For example, the following schema is now readable:

struct FixedArray {
  // This array is large enough that it pushes the offset of another_double above INT16_MAX
  fixed_array:[double:5555];
}

table LargeOffset {
  another_double:double;
  fixed_array:FixedArray;
}

root_type LargeOffset;

(I ran into this bug in the wild when a user showed up with a schema having ~4500 fields, mostly doubles.)

@github-actions
Copy link
Contributor

github-actions bot commented Jul 25, 2025

This pull request is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Jul 25, 2025
@jtbandes
Copy link
Contributor Author

jtbandes commented Jul 25, 2025

Still valid I believe.

@github-actions github-actions bot removed the stale label Jul 26, 2025
@bjornharrtell bjornharrtell self-requested a review August 24, 2025 15:16
bjornharrtell
bjornharrtell previously approved these changes Aug 24, 2025
View reviewed changes
Copy link
Collaborator

@bjornharrtell bjornharrtell left a comment

Choose a reason for hiding this comment

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

LGTM

@bjornharrtell
Copy link
Collaborator

bjornharrtell commented Nov 24, 2025

@jtbandes sorry this never got in, if you have time to resolve the conflict I'll try to get attention to it.

@jtbandes
Copy link
Contributor Author

jtbandes commented Nov 24, 2025

@bjornharrtell done, thanks.

@bjornharrtell bjornharrtell enabled auto-merge (squash) November 27, 2025 10:51
@bjornharrtell bjornharrtell merged commit 84f4b83 into google:master Nov 27, 2025
49 checks passed
@jtbandes jtbandes deleted the ts-vtable-uint16 branch November 27, 2025 16:50
@jtbandes
Copy link
Contributor Author

jtbandes commented Dec 6, 2025

@bjornharrtell thanks so much for pushing this along ❤️ Do you have an idea of when it might be available in a new release?

@bjornharrtell
Copy link
Collaborator

bjornharrtell commented Dec 6, 2025

@jtbandes nope I'm just a janitor. 😜

@BrewTestBot BrewTestBot mentioned this pull request Dec 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bjornharrtell bjornharrtell bjornharrtell approved these changes

Assignees

No one assigned

Labels

typescript

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jtbandes @bjornharrtell