Skip to content

wast-parser: make IsPowerOfTwo support memory64 addresses#2681

Merged
sbc100 merged 1 commit into
WebAssembly:mainfrom
sjamesr:64bit_power_of_two
Jan 14, 2026
Merged

wast-parser: make IsPowerOfTwo support memory64 addresses#2681
sbc100 merged 1 commit into
WebAssembly:mainfrom
sjamesr:64bit_power_of_two

Conversation

@sjamesr

@sjamesr sjamesr commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/wast-parser.cc Outdated
static const size_t kMaxErrorTokenLength = 80;

bool IsPowerOfTwo(uint32_t x) {
bool IsPowerOfTwo(const Address& x) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that can just be Address x, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed

Comment thread src/test-wast-parser.cc
(memory 1)
(func
i32.const 0
i32.load offset=0xFFFF_FFFF_FFFF_FFFF align=0x8000_0000_0000_0000

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this exists in align.wast why doesn't the normal test suite run catch this bug?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If this exists in align.wast why doesn't the normal test suite run catch this bug?

The test came into existence here at WebAssembly/testsuite@4b24564, which I suspect is newer than the version in wabt's copy of the spec tests in third_party.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I see. In that case can you maybe add a comment with a link to that commit and a TODO to remove this test once we update the testsuite?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks

@sbc100 sbc100 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm % testing question

@sjamesr sjamesr force-pushed the 64bit_power_of_two branch from 6f2fd09 to 991e96c Compare January 14, 2026 00:22
@sjamesr sjamesr force-pushed the 64bit_power_of_two branch from 991e96c to 4a492da Compare January 14, 2026 00:56
@sbc100 sbc100 enabled auto-merge (squash) January 14, 2026 01:02
@sbc100 sbc100 merged commit 9ffc939 into WebAssembly:main Jan 14, 2026
17 checks passed
@sjamesr sjamesr deleted the 64bit_power_of_two branch January 14, 2026 01:07
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