Visitar URL original
Bytecode parity by youknowone · Pull Request #7504 · RustPython/RustPython · GitHub
Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix surrogate
  • Loading branch information
youknowone committed Mar 25, 2026
commit cab59e7df9111df5bf2b6e51c61fdc0dbc4be2da
42 changes: 22 additions & 20 deletions crates/codegen/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7622,23 +7622,8 @@ impl Compiler {
self.compile_expr_tstring(tstring)?;
}
ast::Expr::StringLiteral(string) => {
let value = string.value.to_str();
if value.contains(char::REPLACEMENT_CHARACTER) {
let value = string
.value
.iter()
.map(|lit| {
let source = self.source_file.slice(lit.range);
crate::string_parser::parse_string_literal(source, lit.flags.into())
})
.collect();
// might have a surrogate literal; should reparse to be sure
self.emit_load_const(ConstantData::Str { value });
} else {
self.emit_load_const(ConstantData::Str {
value: value.into(),
});
}
let value = self.compile_string_value(string);
self.emit_load_const(ConstantData::Str { value });
}
ast::Expr::BytesLiteral(bytes) => {
let iter = bytes.value.iter().flat_map(|x| x.iter().copied());
Expand Down Expand Up @@ -8569,6 +8554,24 @@ impl Compiler {

// fn block_done()

/// Convert a string literal AST node to Wtf8Buf, handling surrogates correctly.
fn compile_string_value(&self, string: &ast::ExprStringLiteral) -> Wtf8Buf {
let value = string.value.to_str();
if value.contains(char::REPLACEMENT_CHARACTER) {
// Might have a surrogate literal; reparse from source to preserve them
string
.value
.iter()
.map(|lit| {
let source = self.source_file.slice(lit.range);
crate::string_parser::parse_string_literal(source, lit.flags.into())
})
.collect()
} else {
value.into()
}
}
Comment on lines +8557 to +8573

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

This still leaves surrogate-bearing docstrings on the lossy path.

compile_string_value() fixes regular string literals, but docstrings still go through split_doc() and clean_doc(doc.to_str()). So a module/class/function docstring containing an isolated surrogate will still be normalized to U+FFFD, which means literal parity is only partial after this change.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/compile.rs` around lines 8557 - 8573, Docstrings are still
being converted lossily because split_doc() and clean_doc(doc.to_str()) operate
on the lossy UTF-8 string; fix by making docstring handling follow the same
non-lossy path as compile_string_value(): when splitting/cleaning docs, detect
replacement characters and, instead of using doc.to_str(), reparse the original
source range(s) with crate::string_parser::parse_string_literal (or call
compile_string_value on the underlying ast::ExprStringLiteral) to produce a
Wtf8Buf preserving surrogates, then pass that Wtf8Buf into split_doc/clean_doc
logic (or change clean_doc to accept Wtf8Buf) so module/class/function
docstrings retain literal parity with compile_string_value.


fn arg_constant(&mut self, constant: ConstantData) -> oparg::ConstIdx {
let info = self.current_code_info();
info.metadata.consts.insert_full(constant).0.to_u32().into()
Expand Down Expand Up @@ -8598,9 +8601,8 @@ impl Compiler {
}
},
ast::Expr::StringLiteral(s) => {
constants.push(ConstantData::Str {
value: s.value.to_string().into(),
});
let value = self.compile_string_value(s);
constants.push(ConstantData::Str { value });
}
ast::Expr::BytesLiteral(b) => {
constants.push(ConstantData::Bytes {
Expand Down
Loading