From a4a052b351c411987088d05e923cf077edfdea68 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Fri, 1 Nov 2024 07:27:13 -0700 Subject: [PATCH 1/2] transpile: improve error message for `SrcLoc` total ordering test --- c2rust-ast-exporter/src/clang_ast.rs | 30 +++++++++++++++++++++++++++- c2rust-transpile/src/c_ast/mod.rs | 7 ++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/c2rust-ast-exporter/src/clang_ast.rs b/c2rust-ast-exporter/src/clang_ast.rs index 0a4de278ef..2341ee99ee 100644 --- a/c2rust-ast-exporter/src/clang_ast.rs +++ b/c2rust-ast-exporter/src/clang_ast.rs @@ -1,9 +1,10 @@ use serde_bytes::ByteBuf; use serde_cbor::error; -use std; use std::collections::{HashMap, VecDeque}; use std::convert::TryInto; +use std::fmt::{Display, Formatter}; use std::path::{Path, PathBuf}; +use std::{self, fmt}; pub use serde_cbor::value::{from_value, Value}; @@ -31,6 +32,17 @@ pub struct SrcLoc { pub column: u64, } +impl Display for SrcLoc { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + let Self { + fileid, + line, + column, + } = *self; + write!(f, "file_{fileid}:{line}:{column}") + } +} + #[derive(Copy, Debug, Clone, PartialOrd, PartialEq, Ord, Eq)] pub struct SrcSpan { pub fileid: u64, @@ -40,6 +52,22 @@ pub struct SrcSpan { pub end_column: u64, } +impl Display for SrcSpan { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + let Self { + fileid, + begin_line, + begin_column, + end_line, + end_column, + } = *self; + write!( + f, + "file_{fileid}:{begin_line}:{begin_column}-{end_line}:{end_column}" + ) + } +} + impl From for SrcSpan { fn from(loc: SrcLoc) -> Self { Self { diff --git a/c2rust-transpile/src/c_ast/mod.rs b/c2rust-transpile/src/c_ast/mod.rs index 9506eac705..f55be9865f 100644 --- a/c2rust-transpile/src/c_ast/mod.rs +++ b/c2rust-transpile/src/c_ast/mod.rs @@ -2101,7 +2101,12 @@ mod tests { let bc = ctx.compare_src_locs(&b, &c); let ac = ctx.compare_src_locs(&a, &c); if ab == bc { - assert_eq!(ab, ac, "Total order (transitivity) has been violated"); + let [ab, bc, ac] = [ab, bc, ac].map(|ord| match ord { + Ordering::Less => "<", + Ordering::Equal => "==", + Ordering::Greater => ">", + }); + assert_eq!(ab, ac, "Total order (transitivity) has been violated: {a} {ab} {b} and {b} {bc} {c}, but {a} {ac} {c}"); } } } From 9679a3befb9d5d9431ee7db4d967347edb64536d Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Mon, 18 Nov 2024 01:58:56 -0800 Subject: [PATCH 2/2] transpile: fix `curl` transpile breakage while maintaining a total `SrcLoc` order #1128 fixed the non-total (non-transitive) `SrcLoc` ordering, but accidentally broke the `curl` transpilation test in `c2rust-testsuites`, which is tested in CI, but was broken for external contributors PRs (fixed now in #1159). In the `curl` transpilation, a couple of `use` imports (`__sigset_t` and `C2RustUnnamed_4`) were missing, cause the build to fail afterward. I'm still not exactly sure why this fixes the issue while maintaining a transitive, total order, but it passes the total order test and transpiles `curl` correctly now. Hopefully this is a complete fix, and I didn't just fix a one-off error in `curl`. --- c2rust-transpile/src/c_ast/mod.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/c2rust-transpile/src/c_ast/mod.rs b/c2rust-transpile/src/c_ast/mod.rs index f55be9865f..9a7a1f6c1b 100644 --- a/c2rust-transpile/src/c_ast/mod.rs +++ b/c2rust-transpile/src/c_ast/mod.rs @@ -208,9 +208,14 @@ impl TypedAstContext { /// Compare two [`SrcLoc`]s based on their import path pub fn compare_src_locs(&self, a: &SrcLoc, b: &SrcLoc) -> Ordering { + /// Compare without regard to `fileid`. + fn cmp_pos(a: &SrcLoc, b: &SrcLoc) -> Ordering { + (a.line, a.column).cmp(&(b.line, b.column)) + } + use Ordering::*; - let path_a = &self.include_map[self.file_map[a.fileid as usize]]; - let path_b = &self.include_map[self.file_map[b.fileid as usize]]; + let path_a = &self.include_map[self.file_map[a.fileid as usize]][..]; + let path_b = &self.include_map[self.file_map[b.fileid as usize]][..]; // Find the first include that does not match between the two let common_len = path_a.len().min(path_b.len()); @@ -221,22 +226,21 @@ impl TypedAstContext { // Either all parent includes are the same, or the include paths are of different lengths // because .zip() stops when one of the iterators is empty. - let (a, b) = match path_a.len().cmp(&path_b.len()) { + match path_a.len().cmp(&path_b.len()) { Less => { // a has the shorter path, which means b was included in a's file // so extract that include and compare the position to a let b = &path_b[path_a.len()]; - (a, b) + cmp_pos(a, b) } - Equal => (a, b), // a and b have the same include path and are thus in the same file + Equal => a.cmp(b), // a and b have the same include path and are thus in the same file Greater => { // b has the shorter path, which means a was included in b's file // so extract that include and compare the position to b let a = &path_a[path_b.len()]; - (a, b) + cmp_pos(a, b) } - }; - a.cmp(b) + } } pub fn get_file_include_line_number(&self, file: FileId) -> Option {