transform: keep escaped ${ literal in interpolated strings - #28942
Merged
Merged
Conversation
The transform re-parsed the text of every string literal inside a string
interpolation as `${...}` code. The parser has already unescaped that text,
so an escaped `\${name}` next to a real interpolation became a nested
interpolation: undeclared names failed in C (`'err' undeclared`, seen in
ui2/ide), and declared ones were silently substituted
(`'${x} \${x}'` printed `X X`). `\x24{`, `\044{` and raw literals inside an
interpolation hit the same path.
The re-parse was a workaround from before the scanner split nested
interpolations itself (#28331). A string literal the parser produces can no
longer hold a real interpolation, so pass string literals through unchanged
and drop the now-unused recovery helpers.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
v ~/.vmodules/ui2/idefails in C:companion_main_sourcebuilds V source text with a string like"... ${vml_escape(vml_name)}: \${err}') ...". The transform re-parsed the text of everystring literal inside a string interpolation as
${...}code. The parser has alreadyunescaped that text, so an escaped
\${name}was expanded as a real interpolation.Undeclared names fail in C. Declared names are silently substituted:
A string literal on its own (
'\${x}') was fine. The bug needed a real interpolation in thesame string, or the literal nested inside one.
Fix
The re-parse was a workaround from before the scanner split nested interpolations itself
(#28331). A string literal the parser produces can no longer contain a real interpolation, so
transform_exprnow passes string literals through unchanged. The recovery helpers thisleaves unused are removed, along with the
in_string_interp_partflag. The ORM path stilluses
simple_nested_string_interpolationfor SQL tokens and is unchanged.#28416 fixed the
itcase of this same bug on its own ('--\${it}'inside amap). Thattest still passes.
Still open:
transform_nested_if_string_interp_node/..._match_...(and the matchinghelper in
if.v) apply the same kind of text heuristic to a 5-part interpolation, so'\${if ok { ${a} } else { ${b} }}'still printsA. That needs an escaped\${ifor\${matchfollowed by branch-shaped real interpolations. It is left out to keep thischange focused.
Tests
vlib/v/tests/builtin_strings_and_interpolation/string_interpolation_escaped_dollar_test.v:escaped
\${,\x24{,\044{, raw literals and escaped text inside interpolationexpressions stay literal, and real nested interpolation still expands. Fails on master:
C error for the undeclared-name case, assertion failures (
X XvsX ${x}) without it.v test vlib/v/tests/builtin_strings_and_interpolation/: 51/51 pass.vlib/v/compiler_tests/nested_escaped_map_it_interp_test.v: passes.ui2/idenow builds.