feat(string): add String::Split, EscapedStringPy, and rename EscapeString#550
Merged
junrushao merged 2 commits intoapache:mainfrom Apr 14, 2026
Merged
Conversation
…ring to EscapeStringJSON ## Architecture - Rename `EscapeString` -> `EscapeStringJSON` to clarify its JSON-specific escaping behavior (RFC 8259 compliant). - Add deprecated `EscapeString` alias for backward compatibility. - Add `EscapedStringPy` for Python-style string escaping that handles ANSI escape sequences, UTF-8 multibyte characters, and standard C escapes. - Add `String::Split(char delim)` utility returning `vector<string_view>`. ## Public Interfaces - `EscapeStringJSON(const String&)` -- new canonical name for JSON escaping. - `EscapeString(const String&)` -- deprecated, forwards to `EscapeStringJSON`. - `EscapedStringPy(const String&)` -- new Python-style string escaping. - `String::Split(char)` -- new method returning split segments as string_views. ## Behavioral Changes - All internal call sites (`function.h`, `registry.h`, `dataclass.cc`, `json_writer.cc`) updated to use `EscapeStringJSON` directly. - No runtime behavior change: `EscapeStringJSON` is the same implementation as the previous `EscapeString`. ## Docs - Doxygen comments added for all new/renamed functions. ## Tests - No new tests in this commit. ## Untested Edge Cases - `EscapedStringPy` with malformed UTF-8 input (partial multibyte sequences). - `String::Split` with empty strings or strings consisting only of delimiters.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors string escaping by renaming EscapeString to EscapeStringJSON and introducing EscapedStringPy for Python-style representation. It also adds a Split method to the String class. Feedback focuses on improving the robustness and consistency of the new escaping logic, specifically regarding std::isdigit safety, ANSI escape sequence formatting, and UTF-8 validation. Additionally, it is suggested to use EscapedStringPy within ReprPrinter to maintain Python-compatible output.
yzh119
approved these changes
Apr 14, 2026
Member
yzh119
left a comment
yzh119
left a comment
There was a problem hiding this comment.
Do we have unittests for this feature?
## Architecture - `EscapedStringPy` in `string.h` is the Python-style string escaping utility used by `ReprPrinter` for `__repr__` output of TVM FFI objects. - `ReprPrinter` in `dataclass.cc` traverses object graphs and formats string fields via the escaper. ## Public Interfaces No changes to public API signatures. ## Behavioral Changes - Cast `data[j]` to `unsigned char` before `std::isdigit` to avoid undefined behavior when char values are negative (string.h:908). - ANSI escape sequences now render as `\x1b[...` instead of `\u001b[...` for Python repr consistency (string.h:912). - UTF-8 multi-byte decoding now validates continuation bytes `(byte & 0xC0) == 0x80`; malformed sequences fall back to per-byte `\xNN` escaping instead of silently decoding garbage (string.h:957-971). - `ReprPrinter` now calls `EscapedStringPy` instead of `EscapeStringJSON` so that `__repr__` output uses Python-style escaping (dataclass.cc:782,815). ## Tests - Added 7 new GoogleTest cases in `test_string.cc` covering `String::Split`, `EscapeStringJSON`, and `EscapedStringPy` (basic, control chars, ANSI, UTF-8, malformed UTF-8). - All 47 string tests pass. ## Untested Edge Cases - 5-byte and 6-byte overlong UTF-8 sequences (invalid per RFC 3629 but could appear in arbitrary binary data).
Member
Author
|
@yzh119 Unittests added |
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
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.
Summary
EscapeStringtoEscapeStringJSONto clarify its JSON-specific escaping semantics (RFC 8259). A deprecatedEscapeStringalias is retained for backward compatibility.EscapedStringPyfor Python-style string escaping that handles ANSI escape sequences, UTF-8 multibyte characters, and standard C escape sequences (\n,\t,\r,\\,\").String::Split(char delim)utility method that returnsstd::vector<std::string_view>segments.function.h,registry.h,dataclass.cc,json_writer.cc) to use the newEscapeStringJSONname.ReprPrinternow usesEscapedStringPyinstead ofEscapeStringJSONfor proper Python-style__repr__output.Motivation
The existing
EscapeStringfunction was JSON-specific but its name did not convey this. This rename makes intent explicit. The newEscapedStringPyfunction supports Python-style repr output needed for error messages and debugging.String::Splitis a common utility needed across the codebase.Changes
include/tvm/ffi/string.hEscapeString->EscapeStringJSON, add deprecated alias, addEscapedStringPy, addString::Splitinclude/tvm/ffi/string.hunsigned charbeforestd::isdigitto avoid UB; use\x1bfor ANSI escapes; validate UTF-8 continuation bytesinclude/tvm/ffi/function.hEscapeStringJSONinclude/tvm/ffi/reflection/registry.hEscapeStringJSONsrc/ffi/extra/dataclass.ccReprPrinterusesEscapedStringPyfor Python-style repr outputsrc/ffi/extra/json_writer.ccEscapeStringJSONtests/cpp/test_string.ccSplit,EscapeStringJSON,EscapedStringPy(basic, control chars, ANSI, UTF-8, malformed UTF-8)Test plan
String::Splittested with edge cases (empty, boundaries, consecutive delimiters)EscapeStringJSONtested with special chars, backslash, quotes, control charsEscapedStringPytested: basic ASCII, control chars, ANSI sequences, valid UTF-8 (2/3/4-byte), malformed UTF-8