From 58836880a0f77c6a7169a1612751b16808727d02 Mon Sep 17 00:00:00 2001 From: Martin Geisler <mgeisler@google.com> Date: Mon, 4 Apr 2022 10:53:59 +0200 Subject: [PATCH] pdl: fix computation of `SourceLocation` line numbers Before, we would always exit the loop at the first iteration since the `line_starts` passed in start with a zero offset. We should actually search through the line starts to find the last line which starts before the given offset. This also adds new unit tests to cover this. Test: atest pdl_inline_tests Change-Id: I171803822c3cb58deec167a2bc063cbd8f99110f --- tools/pdl/Android.bp | 19 +++++++++++----- tools/pdl/src/ast.rs | 52 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/tools/pdl/Android.bp b/tools/pdl/Android.bp index 50f4ceae399..b1cbcd4aaf5 100644 --- a/tools/pdl/Android.bp +++ b/tools/pdl/Android.bp @@ -8,11 +8,9 @@ package { default_applicable_licenses: ["system_bt_license"], } -rust_binary_host { - name: "pdl", - srcs: [ - "src/main.rs", - ], +rust_defaults { + name: "pdl_defaults", + srcs: ["src/main.rs"], rustlibs: [ "libpest", "libserde", @@ -24,3 +22,14 @@ rust_binary_host { "libpest_derive", ], } + +rust_binary_host { + name: "pdl", + defaults: ["pdl_defaults"], +} + +rust_test_host { + name: "pdl_inline_tests", + defaults: ["pdl_defaults"], + test_suites: ["general-tests"], +} diff --git a/tools/pdl/src/ast.rs b/tools/pdl/src/ast.rs index e25b01a207b..01ce05c6453 100644 --- a/tools/pdl/src/ast.rs +++ b/tools/pdl/src/ast.rs @@ -14,8 +14,11 @@ pub type SourceDatabase = files::SimpleFiles<String, String>; #[derive(Debug, Copy, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)] pub struct SourceLocation { + /// Byte offset into the file (counted from zero). pub offset: usize, + /// Line number (counted from zero). pub line: usize, + /// Column number (counted from zero) pub column: usize, } @@ -176,13 +179,20 @@ pub trait Named<'d> { } impl SourceLocation { + /// Construct a new source location. + /// + /// The `line_starts` indicates the byte offsets where new lines + /// start in the file. The first element should thus be `0` since + /// every file has at least one line starting at offset `0`. pub fn new(offset: usize, line_starts: &[usize]) -> SourceLocation { + let mut loc = SourceLocation { offset, line: 0, column: offset }; for (line, start) in line_starts.iter().enumerate() { - if *start <= offset { - return SourceLocation { offset, line, column: offset - start }; + if *start > offset { + break; } + loc = SourceLocation { offset, line, column: offset - start }; } - unreachable!() + loc } } @@ -299,3 +309,39 @@ impl<'d> Named<'d> for Decl { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn source_location_new() { + let line_starts = &[0, 20, 80, 120, 150]; + assert_eq!( + SourceLocation::new(0, line_starts), + SourceLocation { offset: 0, line: 0, column: 0 } + ); + assert_eq!( + SourceLocation::new(10, line_starts), + SourceLocation { offset: 10, line: 0, column: 10 } + ); + assert_eq!( + SourceLocation::new(50, line_starts), + SourceLocation { offset: 50, line: 1, column: 30 } + ); + assert_eq!( + SourceLocation::new(100, line_starts), + SourceLocation { offset: 100, line: 2, column: 20 } + ); + assert_eq!( + SourceLocation::new(1000, line_starts), + SourceLocation { offset: 1000, line: 4, column: 850 } + ); + } + + #[test] + fn source_location_new_no_crash_with_empty_line_starts() { + let loc = SourceLocation::new(100, &[]); + assert_eq!(loc, SourceLocation { offset: 100, line: 0, column: 100 }); + } +} -- GitLab