Skip to content
Merged
Show file tree
Hide file tree
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
Next Next commit
fix debug range
  • Loading branch information
youknowone committed Dec 30, 2025
commit 69fd874d9a8c3bb7d92c281334dba48bb6fab77e
30 changes: 21 additions & 9 deletions crates/codegen/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,22 @@ enum DoneWithFuture {
Yes,
}

#[derive(Debug, Clone, Default)]
#[derive(Debug, Clone)]
pub struct CompileOpts {
/// How optimized the bytecode output should be; any optimize > 0 does
/// not emit assert statements
pub optimize: u8,
/// Include column info in bytecode (-X no_debug_ranges disables)
pub debug_ranges: bool,
}

impl Default for CompileOpts {
fn default() -> Self {
Self {
optimize: 0,
debug_ranges: true,
}
}
}

#[derive(Debug, Clone, Copy)]
Expand Down Expand Up @@ -859,7 +870,7 @@ impl Compiler {
let pop = self.code_stack.pop();
let stack_top = compiler_unwrap_option(self, pop);
// No parent scope stack to maintain
unwrap_internal(self, stack_top.finalize_code(self.opts.optimize))
unwrap_internal(self, stack_top.finalize_code(&self.opts))
}

/// Push a new fblock
Expand Down Expand Up @@ -1486,7 +1497,9 @@ impl Compiler {
..
}) => self.compile_for(target, iter, body, orelse, *is_async)?,
Stmt::Match(StmtMatch { subject, cases, .. }) => self.compile_match(subject, cases)?,
Stmt::Raise(StmtRaise { exc, cause, .. }) => {
Stmt::Raise(StmtRaise {
exc, cause, range, ..
}) => {
let kind = match exc {
Some(value) => {
self.compile_expression(value)?;
Expand All @@ -1500,6 +1513,7 @@ impl Compiler {
}
None => bytecode::RaiseKind::Reraise,
};
self.set_source_range(*range);
emit!(self, Instruction::Raise { kind });
}
Stmt::Try(StmtTry {
Expand Down Expand Up @@ -5639,17 +5653,15 @@ impl Compiler {
// Low level helper functions:
fn _emit(&mut self, instr: Instruction, arg: OpArg, target: BlockIdx) {
let range = self.current_source_range;
let location = self
.source_file
.to_source_code()
.source_location(range.start(), PositionEncoding::Utf8);
// TODO: insert source filename
let source = self.source_file.to_source_code();
let location = source.source_location(range.start(), PositionEncoding::Utf8);
let end_location = source.source_location(range.end(), PositionEncoding::Utf8);
self.current_block().instructions.push(ir::InstructionInfo {
instr,
arg,
target,
location,
// range,
end_location,
});
}

Expand Down
87 changes: 55 additions & 32 deletions crates/codegen/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,8 @@ pub struct InstructionInfo {
pub instr: Instruction,
pub arg: OpArg,
pub target: BlockIdx,
// pub range: TextRange,
pub location: SourceLocation,
// TODO: end_location for debug ranges
pub end_location: SourceLocation,
}

// spell-checker:ignore petgraph
Expand Down Expand Up @@ -133,8 +132,11 @@ pub struct CodeInfo {
}

impl CodeInfo {
pub fn finalize_code(mut self, optimize: u8) -> crate::InternalResult<CodeObject> {
if optimize > 0 {
pub fn finalize_code(
mut self,
opts: &crate::compile::CompileOpts,
) -> crate::InternalResult<CodeObject> {
if opts.optimize > 0 {
self.dce();
}

Expand Down Expand Up @@ -198,7 +200,10 @@ impl CodeInfo {
*arg = new_arg;
}
let (extras, lo_arg) = arg.split();
locations.extend(core::iter::repeat_n(info.location, arg.instr_size()));
locations.extend(core::iter::repeat_n(
(info.location, info.end_location),
arg.instr_size(),
));
instructions.extend(
extras
.map(|byte| CodeUnit::new(Instruction::ExtendedArg, byte))
Expand All @@ -217,7 +222,11 @@ impl CodeInfo {
}

// Generate linetable from locations
let linetable = generate_linetable(&locations, first_line_number.get() as i32);
let linetable = generate_linetable(
&locations,
first_line_number.get() as i32,
opts.debug_ranges,
);

Ok(CodeObject {
flags,
Expand Down Expand Up @@ -412,7 +421,11 @@ fn iter_blocks(blocks: &[Block]) -> impl Iterator<Item = (BlockIdx, &Block)> + '
}

/// Generate CPython 3.11+ format linetable from source locations
fn generate_linetable(locations: &[SourceLocation], first_line: i32) -> Box<[u8]> {
fn generate_linetable(
locations: &[(SourceLocation, SourceLocation)],
first_line: i32,
debug_ranges: bool,
) -> Box<[u8]> {
if locations.is_empty() {
return Box::new([]);
}
Expand All @@ -424,7 +437,7 @@ fn generate_linetable(locations: &[SourceLocation], first_line: i32) -> Box<[u8]
let mut i = 0;

while i < locations.len() {
let loc = &locations[i];
let (loc, end_loc) = &locations[i];

// Count consecutive instructions with the same location
let mut length = 1;
Expand All @@ -436,18 +449,33 @@ fn generate_linetable(locations: &[SourceLocation], first_line: i32) -> Box<[u8]
while length > 0 {
let entry_length = length.min(8);

// Get line and column information
// SourceLocation always has row and column (both are OneIndexed)
// Get line information
let line = loc.line.get() as i32;
let col = loc.character_offset.to_zero_indexed() as i32;

let end_line = end_loc.line.get() as i32;
let line_delta = line - prev_line;
let end_line_delta = end_line - line;

// Choose the appropriate encoding based on line delta and column info
// Note: SourceLocation always has valid column, so we never get NO_COLUMNS case
if line_delta == 0 {
let end_col = col; // Use same column for end (no range info available)
// When debug_ranges is disabled, only emit line info (NoColumns format)
if !debug_ranges {
// NoColumns format (code 13): line info only, no column data
linetable.push(
0x80 | ((PyCodeLocationInfoKind::NoColumns as u8) << 3)
| ((entry_length - 1) as u8),
);
write_signed_varint(&mut linetable, line_delta);

prev_line = line;
length -= entry_length;
i += entry_length;
continue;
}

// Get column information (only when debug_ranges is enabled)
let col = loc.character_offset.to_zero_indexed() as i32;
let end_col = end_loc.character_offset.to_zero_indexed() as i32;

// Choose the appropriate encoding based on line delta and column info
if line_delta == 0 && end_line_delta == 0 {
if col < 80 && end_col - col < 16 && end_col >= col {
// Short form (codes 0-9) for common cases
let code = (col / 8).min(9) as u8; // Short0 to Short9
Expand All @@ -470,42 +498,37 @@ fn generate_linetable(locations: &[SourceLocation], first_line: i32) -> Box<[u8]
);
write_signed_varint(&mut linetable, 0); // line_delta = 0
write_varint(&mut linetable, 0); // end_line delta = 0
write_varint(&mut linetable, (col as u32) + 1); // column + 1 for encoding
write_varint(&mut linetable, (end_col as u32) + 1); // end_col + 1
write_varint(&mut linetable, (col as u32) + 1);
write_varint(&mut linetable, (end_col as u32) + 1);
}
} else if line_delta > 0 && line_delta < 3
/* && column.is_some() */
{
} else if line_delta > 0 && line_delta < 3 && end_line_delta == 0 {
// One-line form (codes 11-12) for line deltas 1-2
let end_col = col; // Use same column for end

if col < 128 && end_col < 128 {
let code = (PyCodeLocationInfoKind::OneLine0 as u8) + (line_delta as u8); // 11 for delta=1, 12 for delta=2
let code = (PyCodeLocationInfoKind::OneLine0 as u8) + (line_delta as u8);
linetable.push(0x80 | (code << 3) | ((entry_length - 1) as u8));
linetable.push(col as u8);
linetable.push(end_col as u8);
} else {
// Long form for columns >= 128 or negative line delta
// Long form for columns >= 128
linetable.push(
0x80 | ((PyCodeLocationInfoKind::Long as u8) << 3)
| ((entry_length - 1) as u8),
);
write_signed_varint(&mut linetable, line_delta);
write_varint(&mut linetable, 0); // end_line delta = 0
write_varint(&mut linetable, (col as u32) + 1); // column + 1 for encoding
write_varint(&mut linetable, (end_col as u32) + 1); // end_col + 1
write_varint(&mut linetable, (col as u32) + 1);
write_varint(&mut linetable, (end_col as u32) + 1);
}
} else {
// Long form (code 14) for all other cases
// This handles: line_delta < 0, line_delta >= 3, or columns >= 128
let end_col = col; // Use same column for end
// Handles: line_delta < 0, line_delta >= 3, multi-line spans, or columns >= 128
linetable.push(
0x80 | ((PyCodeLocationInfoKind::Long as u8) << 3) | ((entry_length - 1) as u8),
);
write_signed_varint(&mut linetable, line_delta);
write_varint(&mut linetable, 0); // end_line delta = 0
write_varint(&mut linetable, (col as u32) + 1); // column + 1 for encoding
write_varint(&mut linetable, (end_col as u32) + 1); // end_col + 1
write_varint(&mut linetable, end_line_delta as u32);
write_varint(&mut linetable, (col as u32) + 1);
write_varint(&mut linetable, (end_col as u32) + 1);
}

prev_line = line;
Expand Down
6 changes: 3 additions & 3 deletions crates/compiler-core/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ impl ConstantBag for BasicBag {
#[derive(Clone)]
pub struct CodeObject<C: Constant = ConstantData> {
pub instructions: CodeUnits,
pub locations: Box<[SourceLocation]>,
pub locations: Box<[(SourceLocation, SourceLocation)]>,
pub flags: CodeFlags,
/// Number of positional-only arguments
pub posonlyarg_count: u32,
Expand Down Expand Up @@ -1483,14 +1483,14 @@ impl<C: Constant> CodeObject<C> {
level: usize,
) -> fmt::Result {
let label_targets = self.label_targets();
let line_digits = (3).max(self.locations.last().unwrap().line.digits().get());
let line_digits = (3).max(self.locations.last().unwrap().0.line.digits().get());
let offset_digits = (4).max(1 + self.instructions.len().ilog10() as usize);
let mut last_line = OneIndexed::MAX;
let mut arg_state = OpArgState::default();
for (offset, &instruction) in self.instructions.iter().enumerate() {
let (instruction, arg) = arg_state.get(instruction);
// optional line number
let line = self.locations[offset].line;
let line = self.locations[offset].0.line;
if line != last_line {
if last_line != OneIndexed::MAX {
writeln!(f)?;
Expand Down
19 changes: 13 additions & 6 deletions crates/compiler-core/src/marshal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,17 @@ pub fn deserialize_code<R: Read, Bag: ConstantBag>(
let len = rdr.read_u32()?;
let locations = (0..len)
.map(|_| {
Ok(SourceLocation {
let start = SourceLocation {
line: OneIndexed::new(rdr.read_u32()? as _).ok_or(MarshalError::InvalidLocation)?,
character_offset: OneIndexed::from_zero_indexed(rdr.read_u32()? as _),
})
};
let end = SourceLocation {
line: OneIndexed::new(rdr.read_u32()? as _).ok_or(MarshalError::InvalidLocation)?,
character_offset: OneIndexed::from_zero_indexed(rdr.read_u32()? as _),
};
Ok((start, end))
})
.collect::<Result<Box<[SourceLocation]>>>()?;
.collect::<Result<Box<[(SourceLocation, SourceLocation)]>>>()?;

Comment on lines 190 to 204

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 | 🟠 Major

Marshal format for locations changed without bumping FORMAT_VERSION

The code at lines 190-204 (deserialize) and 649-661 (serialize) is now symmetric and correctly treats each location entry as a (start, end) pair, reading and writing four u32 values per entry (start line, start column, end line, end column). However, FORMAT_VERSION remains at 4 with no version-based conditional logic.

If this represents a format change from prior versions (where locations might have had a different layout), then any marshalled code produced with earlier versions of FORMAT_VERSION 4 will be incompatible with the current reader—and vice versa. Existing frozen stdlib artifacts, cached bytecode, or external tools relying on the old layout would fail to deserialize.

To address this:

  • Bump FORMAT_VERSION and guard the (start, end) tuple handling on the new version, or
  • Add backward-compat deserialization that can handle both old and new layouts
  • Document whether marshal format 4 is intended to be stable across this change

Otherwise, if locations always had this layout, this concern does not apply.

🤖 Prompt for AI Agents
In crates/compiler-core/src/marshal.rs around lines 190-204 (reader) and 649-661
(writer), the marshal layout for `locations` was changed to a `(start, end)`
tuple of four u32s but FORMAT_VERSION was not bumped or handled; either bump
FORMAT_VERSION and gate the new read/write behavior on the new version, or add
backward-compatible logic: detect the FORMAT_VERSION during deserialization and
branch to the old layout reader when the version is older, and when serializing
emit the format corresponding to the chosen FORMAT_VERSION; update the serialize
code at 649-661 to be conditional on version, update any tests/fixtures and the
format documentation, and ensure unit tests cover both old and new
deserialization paths.

let flags = CodeFlags::from_bits_truncate(rdr.read_u16()?);

Expand Down Expand Up @@ -648,9 +653,11 @@ pub fn serialize_code<W: Write, C: Constant>(buf: &mut W, code: &CodeObject<C>)
buf.write_slice(instructions_bytes);

write_len(buf, code.locations.len());
for loc in &*code.locations {
buf.write_u32(loc.line.get() as _);
buf.write_u32(loc.character_offset.to_zero_indexed() as _);
for (start, end) in &*code.locations {
buf.write_u32(start.line.get() as _);
buf.write_u32(start.character_offset.to_zero_indexed() as _);
buf.write_u32(end.line.get() as _);
buf.write_u32(end.character_offset.to_zero_indexed() as _);
}

buf.write_u16(code.flags.bits());
Expand Down
20 changes: 11 additions & 9 deletions crates/vm/src/builtins/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,20 +467,22 @@ impl Constructor for PyCode {
.collect::<Vec<_>>()
.into_boxed_slice();

// Create locations
// Create locations (start and end pairs)
let row = if args.firstlineno > 0 {
OneIndexed::new(args.firstlineno as usize).unwrap_or(OneIndexed::MIN)
} else {
OneIndexed::MIN
};
let locations: Box<[rustpython_compiler_core::SourceLocation]> = vec![
rustpython_compiler_core::SourceLocation {
line: row,
character_offset: OneIndexed::from_zero_indexed(0),
};
instructions.len()
]
.into_boxed_slice();
let loc = rustpython_compiler_core::SourceLocation {
line: row,
character_offset: OneIndexed::from_zero_indexed(0),
};
let locations: Box<
[(
rustpython_compiler_core::SourceLocation,
rustpython_compiler_core::SourceLocation,
)],
> = vec![(loc, loc); instructions.len()].into_boxed_slice();

// Build the CodeObject
let code = CodeObject {
Expand Down
4 changes: 2 additions & 2 deletions crates/vm/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl Frame {
}

pub fn current_location(&self) -> SourceLocation {
self.code.locations[self.lasti() as usize - 1]
self.code.locations[self.lasti() as usize - 1].0
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

pub fn lasti(&self) -> u32 {
Expand Down Expand Up @@ -385,7 +385,7 @@ impl ExecutingFrame<'_> {
// 2. Add new entry with current execution position (filename, lineno, code_object) to traceback.
// 3. Unwind block stack till appropriate handler is found.

let loc = frame.code.locations[idx];
let (loc, _end_loc) = frame.code.locations[idx];
let next = exception.__traceback__();
let new_traceback = PyTraceback::new(
next,
Expand Down
1 change: 1 addition & 0 deletions crates/vm/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ impl VirtualMachine {
pub fn compile_opts(&self) -> crate::compiler::CompileOpts {
crate::compiler::CompileOpts {
optimize: self.state.config.settings.optimize,
debug_ranges: self.state.config.settings.code_debug_ranges,
}
}

Expand Down
4 changes: 3 additions & 1 deletion crates/vm/src/vm/setting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ pub struct Settings {
// int tracemalloc;
// int perf_profiling;
// int import_time;
// int code_debug_ranges;
/// -X no_debug_ranges: disable column info in bytecode
pub code_debug_ranges: bool,
// int show_ref_count;
// int dump_refs;
// wchar_t *dump_refs_file;
Expand Down Expand Up @@ -192,6 +193,7 @@ impl Default for Settings {
argv: vec![],
hash_seed: None,
faulthandler: false,
code_debug_ranges: true,
buffered_stdio: true,
check_hash_pycs_mode: CheckHashPycsMode::Default,
allow_external_library: cfg!(feature = "importlib"),
Expand Down
5 changes: 4 additions & 1 deletion examples/dis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ fn main() -> Result<(), lexopt::Error> {
return Err("expected at least one argument".into());
}

let opts = compiler::CompileOpts { optimize };
let opts = compiler::CompileOpts {
optimize,
debug_ranges: true,
};

for script in &scripts {
if script.exists() && script.is_file() {
Expand Down
Loading