GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Handle parentheses inside strings in lit check updater by Blue1977 · Pull Request #9372 · WebAssembly/binaryen

Conversation

Blue1977 commented Nov 9, 2026

Copy link

Copy Markdown

Description

This PR fixes an issue in scripts/update_lit_checks.py where the lit check updater incorrectly counts unbalanced parentheses inside strings (such as "hello(" or "hello)") as part of the WebAssembly syntax structure. This bug causes CHECK assertions to map incorrectly to subsequent functions inside data segments.

Solution

Modified find_end to handle string boundaries and escaped characters by implementing in_string and escaped state tracking. Parentheses inside string literals are now successfully skipped, ensuring accurate structure parsing.

Validation Results (WSL Ubuntu)

Local regression test suite executed using test/unit/test_update_lit_checks_strings.py:

before log status: 5 expected subcase failures detected (string='hello(', hello)', hello((), hello\"(, hello\\().
after log status: OK (Ran 1 test containing all subcases successfully).

No issues or PR numbers are linked as this stems from an internal code FIXME.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gemini-code-assist[bot] reacted with eyes emoji

All reactions

👀 1 reaction


          Handle parentheses inside strings in lit check updater

220e9ad

Blue1977 requested a review from a team as a code owner

November 9, 2026 06:13

tlively requested review from tlively and removed request for a team

November 9, 2026 06:13

gemini-code-assist Bot reviewed

Nov 9, 2026

View reviewed changes

gemini-code-assist Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Code Review

This pull request updates scripts/update_lit_checks.py to correctly handle parentheses nested inside string literals, including escaped quotes and backslashes, and adds a comprehensive unit test suite to verify this behavior. A review comment identifies an off-by-one bug in the loop termination logic of find_end that could truncate the closing parenthesis if it is the last character in the string, and provides a suggestion to fix it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

scripts/update_lit_checks.py

Comment on lines 140 to 156

		for end in range(start + 1, len(module)):
		if depth == 0:
		break
		if in_string:
		if escaped:
		escaped = False
		elif module[end] == '\\':
		escaped = True
		elif module[end] == '"':
		in_string = False
		elif module[end] == '"':
		in_string = True
		elif module[end] == '(':
		depth += 1
		elif module[end] == ')':
		depth -= 1
		return end

gemini-code-assist Bot Nov 9, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

There is an off-by-one bug in the loop termination logic. If the matching closing parenthesis is the very last character of the module string, the loop terminates naturally without executing the next iteration where depth == 0 is checked. As a result, end is returned as len(module) - 1 instead of len(module), which incorrectly truncates the closing parenthesis when slicing.

We can fix this and simplify the logic by checking if depth == 0 immediately after decrementing it, and returning end + 1 directly from within the loop.

Suggested change

	for end in range(start + 1, len(module)):
	if depth == 0:
	break
	if in_string:
	if escaped:
	escaped = False
	elif module[end] == '\\':
	escaped = True
	elif module[end] == '"':
	in_string = False
	elif module[end] == '"':
	in_string = True
	elif module[end] == '(':
	depth += 1
	elif module[end] == ')':
	depth -= 1
	return end
	for end in range(start + 1, len(module)):
	if in_string:
	if escaped:
	escaped = False
	elif module[end] == '\\':
	escaped = True
	elif module[end] == '"':
	in_string = False
	elif module[end] == '"':
	in_string = True
	elif module[end] == '(':
	depth += 1
	elif module[end] == ')':
	depth -= 1
	if depth == 0:
	return end + 1
	return len(module)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

tlively Nov 9, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sounds like it's worth fixing this at the same time.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

tlively reviewed

Nov 9, 2026

View reviewed changes

tlively left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for the PR!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/unit/test_update_lit_checks_strings.py

tlively Nov 9, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Rather than a Python unit test, let's just have a small .wast lit test that contains parens inside strings and make sure that the auto-update script can properly generate output for it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

scripts/update_lit_checks.py

Comment on lines 140 to 156

		for end in range(start + 1, len(module)):
		if depth == 0:
		break
		if in_string:
		if escaped:
		escaped = False
		elif module[end] == '\\':
		escaped = True
		elif module[end] == '"':
		in_string = False
		elif module[end] == '"':
		in_string = True
		elif module[end] == '(':
		depth += 1
		elif module[end] == ')':
		depth -= 1
		return end

tlively Nov 9, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sounds like it's worth fixing this at the same time.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

This branch has not been deployed

No deployments

Reviewers

tlively

tlively left review comments

+1 more reviewer

gemini-code-assist[bot]

gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.