Skip to content

Commit e75d601

Browse files
authored
Remove extraneous separator from paths with arguments (#1340)
`compute_size_of_ty()` constructs a path using `path_segment_with_args()`, so that e.g. `sizeof(int)` gets translated to `::core::mem::size_of::<libc::c_int>()`. But this code in ast-builder was adding an extra separator token to the path when a path segment has an argument. The resulting token stream for the call example given before thus corresponded to `::core::mem::size_of::<libc::c_int> :: ()`. You can verify this by adding a debug print log to show the converted value of a conditional and then translating code like `(sizeof(int) > 0) ? 0 : 0)`. I haven't dug into why, but (unless I'm mistaken about what's doing what here) `prettyplease` masks this bug by not including the syntactically incorrect trailing separator in its string output. The incorrect syntax can be exposed when converting the invalid path to a token stream. Or, put another way, passing such expressions to a macro exposes the bad syntax. Without this patch, translating ```c #include <stddef.h> #include <stdio.h> struct s { int p[512]; }; int foo(unsigned char x) { return offsetof(struct s, p[sizeof(int) + x]); } int main(int argc, char** argv) { printf("%d\n", foo(argc)); return 0; } ``` gives `foo` as ```rs #[no_mangle] pub unsafe extern "C" fn foo(mut x: libc::c_uchar) -> libc::c_int { return offset_of!( s, p[(::core::mem::size_of:: < libc::c_int > :: () as libc::c_ulong) // ^^ this guy right here! .wrapping_add(x as libc::c_ulong) as usize] ) as libc::c_ulong as libc::c_int; } ``` Anyways, it appears that every use of `path_segment_with_args` in c2rust comes at the end of a path. I don't know if the extra separator would be needed for segments that are not at the end. Maybe there are other ways of getting paths with args that I didn't look for? I tried running the `c2rust-testsuite` but it seems to want a very specific machine config. So testing in CI it is...
2 parents 39907f6 + c6376e6 commit e75d601

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

c2rust-ast-builder/src/builder.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -315,13 +315,7 @@ impl<S: Make<PathSegment>> Make<Path> for Vec<S> {
315315
fn make(self, mk: &Builder) -> Path {
316316
let mut segments = Punctuated::new();
317317
for s in self {
318-
let segment = s.make(mk);
319-
let has_params = !segment.arguments.is_empty();
320-
segments.push(segment);
321-
// separate params from their segment with ::
322-
if has_params {
323-
segments.push_punct(Default::default());
324-
}
318+
segments.push(s.make(mk));
325319
}
326320
Path {
327321
leading_colon: None,

c2rust-transpile/tests/tokens.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
use c2rust_ast_builder::mk;
2+
use syn::__private::ToTokens;
3+
4+
#[test]
5+
fn test_tokenize() {
6+
let tys = vec![mk().path_ty(vec!["t"])];
7+
let args = mk().angle_bracketed_args(tys);
8+
let path_segment = mk().path_segment_with_args("x", args);
9+
assert_eq!(path_segment.to_token_stream().to_string(), "x :: < t >");
10+
11+
let path = mk().path(vec![path_segment]);
12+
assert_eq!(path.to_token_stream().to_string(), "x :: < t >");
13+
}

0 commit comments

Comments
 (0)