Skip to content

Commit

Permalink
transpile: fix curl transpile breakage while maintaining a total `S…
Browse files Browse the repository at this point in the history
…rcLoc` order (#1163)

#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`.

* 9679a3b is the real fix.
* a4a052b are generally useful for testing and debugging.
  • Loading branch information
kkysen authored Nov 18, 2024
2 parents a3a47ba + 9679a3b commit 3b8a6cd
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 10 deletions.
30 changes: 29 additions & 1 deletion c2rust-ast-exporter/src/clang_ast.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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,
Expand All @@ -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<SrcLoc> for SrcSpan {
fn from(loc: SrcLoc) -> Self {
Self {
Expand Down
27 changes: 18 additions & 9 deletions c2rust-transpile/src/c_ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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<u64> {
Expand Down Expand Up @@ -2101,7 +2105,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}");
}
}
}
Expand Down

0 comments on commit 3b8a6cd

Please sign in to comment.