cleanup AST builder functionality (#1350)

kkysen · web-flow · commit 67cfbc5a7169 · 2025-09-28T03:58:56.000-04:00
- remove unused function &amp; impl
  - `impl&lt;'a&gt; Make&lt;Unsafety&gt; for &amp;'a str`
  - `impl&lt;'a&gt; Make&lt;Constness&gt; for &amp;'a str`
  - `impl Make&lt;TokenStream&gt; for Vec&lt;String&gt;`
  - `impl Make&lt;PathArguments&gt; for ParenthesizedGenericArguments`
  - `fn parenthesized_args`
- refactor `mk().vis()` usage so we can remove `impl&lt;'a&gt;
Make&lt;Visibility&gt; for &amp;'a str`
- refactor `mk().set_mutbl()` usage so we can remove `impl&lt;'a&gt;
Make&lt;Mutability&gt; for &amp;'a str`
- refactor `mk().unary_expr()` usage so we can remove `impl&lt;'a&gt;
Make&lt;UnOp&gt; for &amp;'a str`
- unify `impl Make&lt;TokenStream&gt; for Vec&lt;&amp;str&gt;`, `impl Make&lt;TokenStream&gt;
for Vec&lt;u64&gt;`, and `impl Make&lt;TokenStream&gt; for Vec&lt;Meta&gt;` functionality
to use `fn comma_separated`
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/c2rust-ast-builder/Cargo.toml b/c2rust-ast-builder/Cargo.toml
@@ -12,5 +12,6 @@ keywords.workspace = true
 categories.workspace = true
 
 [dependencies]
+itertools = "0.10.0"
 proc-macro2 = { version = "1.0", features = ["span-locations"]}
 syn = { version = "2.0", features = ["full", "extra-traits", "printing", "clone-impls"]}
diff --git a/c2rust-ast-builder/src/builder.rs b/c2rust-ast-builder/src/builder.rs
@@ -2,6 +2,7 @@
 
 use std::str;
 
+use itertools::intersperse;
 use proc_macro2::{Literal, Punct, Spacing, Span, TokenStream, TokenTree};
 use std::default::Default;
 use std::iter::FromIterator;
@@ -161,6 +162,16 @@ fn punct_box<T, P: Default>(x: Vec<Box<T>>) -> Punctuated<T, P> {
     Punctuated::from_iter(x.into_iter().map(|x| *x))
 }
 
+fn comma_separated<I, T>(items: I) -> TokenStream
+where
+    I: Iterator<Item = T>,
+    T: ToTokens + Clone,
+{
+    let items = items.map(|items| items.to_token_stream());
+    let comma = TokenTree::Punct(Punct::new(',', Spacing::Alone)).into_token_stream();
+    intersperse(items, comma).collect()
+}
+
 pub trait Make<T> {
     fn make(self, mk: &Builder) -> T;
 }
@@ -208,28 +219,6 @@ impl<'a> Make<Path> for &'a str {
     }
 }
 
-impl<'a> Make<Visibility> for &'a str {
-    fn make(self, mk_: &Builder) -> Visibility {
-        match self {
-            "pub" => Visibility::Public(Token![pub](mk_.span)),
-            "priv" | "" | "inherit" => Visibility::Inherited,
-            "pub(crate)" => Visibility::Restricted(VisRestricted {
-                pub_token: Token![pub](mk_.span),
-                paren_token: token::Paren(mk_.span),
-                in_token: None,
-                path: Box::new(mk().path("crate")),
-            }),
-            "pub(super)" => Visibility::Restricted(VisRestricted {
-                pub_token: Token![pub](mk_.span),
-                paren_token: token::Paren(mk_.span),
-                in_token: None,
-                path: Box::new(mk().path("super")),
-            }),
-            _ => panic!("unrecognized string for Visibility: {:?}", self),
-        }
-    }
-}
-
 impl<'a> Make<Abi> for &'a str {
     fn make(self, mk: &Builder) -> Abi {
         Abi {
@@ -252,47 +241,6 @@ impl Make<Extern> for Abi {
     }
 }
 
-impl<'a> Make<Mutability> for &'a str {
-    fn make(self, _mk: &Builder) -> Mutability {
-        match self {
-            "" | "imm" | "immut" | "immutable" => Mutability::Immutable,
-            "mut" | "mutable" => Mutability::Mutable,
-            _ => panic!("unrecognized string for Mutability: {:?}", self),
-        }
-    }
-}
-
-impl<'a> Make<Unsafety> for &'a str {
-    fn make(self, _mk: &Builder) -> Unsafety {
-        match self {
-            "" | "safe" | "normal" => Unsafety::Normal,
-            "unsafe" => Unsafety::Unsafe,
-            _ => panic!("unrecognized string for Unsafety: {:?}", self),
-        }
-    }
-}
-
-impl<'a> Make<Constness> for &'a str {
-    fn make(self, _mk: &Builder) -> Constness {
-        match self {
-            "" | "normal" | "not-const" => Constness::NotConst,
-            "const" => Constness::Const,
-            _ => panic!("unrecognized string for Constness: {:?}", self),
-        }
-    }
-}
-
-impl<'a> Make<UnOp> for &'a str {
-    fn make(self, _mk: &Builder) -> UnOp {
-        match self {
-            "deref" | "*" => UnOp::Deref(Default::default()),
-            "not" | "!" => UnOp::Not(Default::default()),
-            "neg" | "-" => UnOp::Neg(Default::default()),
-            _ => panic!("unrecognized string for UnOp: {:?}", self),
-        }
-    }
-}
-
 impl<I: Make<Ident>> Make<Lifetime> for I {
     fn make(self, mk: &Builder) -> Lifetime {
         Lifetime {
@@ -330,82 +278,21 @@ impl Make<TokenStream> for Vec<TokenTree> {
     }
 }
 
-impl Make<TokenStream> for Vec<String> {
-    fn make(self, _mk: &Builder) -> TokenStream {
-        let mut tokens = vec![];
-        let mut first = true;
-
-        for s in self {
-            if !first {
-                tokens.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
-            } else {
-                first = false;
-            }
-
-            tokens.push(TokenTree::Ident(Ident::new(&s, Span::call_site())));
-        }
-
-        tokens.into_iter().collect::<TokenStream>()
-    }
-}
-
 impl Make<TokenStream> for Vec<&str> {
     fn make(self, _mk: &Builder) -> TokenStream {
-        let mut tokens = vec![];
-        let mut first = true;
-
-        for s in self {
-            if !first {
-                tokens.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
-            } else {
-                first = false;
-            }
-
-            tokens.push(TokenTree::Ident(Ident::new(s, Span::call_site())));
-        }
-
-        tokens.into_iter().collect::<TokenStream>()
+        comma_separated(self.iter().map(|&s| Ident::new(s, Span::call_site())))
     }
 }
 
 impl Make<TokenStream> for Vec<u64> {
     fn make(self, _mk: &Builder) -> TokenStream {
-        let mut tokens = vec![];
-        let mut first = true;
-
-        for s in self {
-            if !first {
-                tokens.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
-            } else {
-                first = false;
-            }
-
-            tokens.push(TokenTree::Literal(Literal::u64_unsuffixed(s)));
-        }
-
-        tokens.into_iter().collect::<TokenStream>()
+        comma_separated(self.iter().map(|&s| Literal::u64_unsuffixed(s)))
     }
 }
 
 impl Make<TokenStream> for Vec<Meta> {
     fn make(self, _mk: &Builder) -> TokenStream {
-        let mut tokens = TokenStream::new();
-
-        let mut first = true;
-
-        for meta in self {
-            if !first {
-                let tt = TokenTree::Punct(Punct::new(',', Spacing::Alone));
-
-                tokens.extend(vec![tt]);
-            } else {
-                first = false;
-            }
-
-            meta.to_tokens(&mut tokens);
-        }
-
-        tokens
+        comma_separated(self.iter())
     }
 }
 
@@ -415,12 +302,6 @@ impl Make<PathArguments> for AngleBracketedGenericArguments {
     }
 }
 
-impl Make<PathArguments> for ParenthesizedGenericArguments {
-    fn make(self, _mk: &Builder) -> PathArguments {
-        PathArguments::Parenthesized(self)
-    }
-}
-
 impl Make<GenericArgument> for Box<Type> {
     fn make(self, _mk: &Builder) -> GenericArgument {
         GenericArgument::Type(*self)
@@ -661,14 +542,6 @@ impl Builder {
         }
     }
 
-    pub fn parenthesized_args(self, tys: Vec<Box<Type>>) -> ParenthesizedGenericArguments {
-        ParenthesizedGenericArguments {
-            paren_token: token::Paren(self.span),
-            inputs: punct_box(tys),
-            output: ReturnType::Default,
-        }
-    }
-
     pub fn angle_bracketed_args<A>(self, args: Vec<A>) -> AngleBracketedGenericArguments
     where
         A: Make<GenericArgument>,
diff --git a/c2rust-transpile/src/cfg/structures.rs b/c2rust-transpile/src/cfg/structures.rs
@@ -667,12 +667,13 @@ impl StructureState {
 ///   * Negating something of the form `!<expr>` produces `<expr>`
 ///
 fn not(bool_expr: &Expr) -> Box<Expr> {
+    use syn::UnOp;
     match *bool_expr {
         Expr::Unary(ExprUnary {
-            op: syn::UnOp::Not(_),
+            op: UnOp::Not(_),
             ref expr,
             ..
         }) => Box::new(unparen(expr).clone()),
-        _ => mk().unary_expr("!", Box::new(bool_expr.clone())),
+        _ => mk().unary_expr(UnOp::Not(Default::default()), Box::new(bool_expr.clone())),
     }
 }
diff --git a/c2rust-transpile/src/translator/literals.rs b/c2rust-transpile/src/translator/literals.rs
@@ -108,7 +108,7 @@ impl<'c> Translation<'c> {
                         // Fallback for characters outside of the valid Unicode range
                         if (val as i32) < 0 {
                             mk().unary_expr(
-                                "-",
+                                UnOp::Neg(Default::default()),
                                 mk().lit_expr(
                                     mk().int_lit((val as i32).unsigned_abs() as u128, "i32"),
                                 ),
@@ -170,7 +170,7 @@ impl<'c> Translation<'c> {
                 Ok(WithStmts::new_unsafe_val(transmute_expr(
                     mk().array_ty(u8_ty, width_lit),
                     self.convert_type(ty.ctype)?,
-                    mk().unary_expr("*", mk().lit_expr(val)),
+                    mk().unary_expr(UnOp::Deref(Default::default()), mk().lit_expr(val)),
                 )))
             }
         }
diff --git a/c2rust-transpile/src/translator/mod.rs b/c2rust-transpile/src/translator/mod.rs
@@ -1008,7 +1008,7 @@ fn make_submodule(
         items.push(mk().extern_("C").foreign_items(foreign_items));
     }
 
-    let module_builder = mk().vis("pub");
+    let module_builder = mk().pub_();
     let module_builder = if reorganize_definitions {
         let file_path_str = file_path.map_or(mod_name.as_str(), |path| {
             path.to_str().expect("Found invalid unicode")
@@ -1929,16 +1929,15 @@ impl<'c> Translation<'c> {
                     .expect("Variables should already be renamed");
                 let ConvertedVariable { ty, mutbl, init: _ } =
                     self.convert_variable(ctx.static_(), None, typ)?;
-                // When putting extern statics into submodules, they need to be public to be accessible
-                let visibility = if self.tcfg.reorganize_definitions {
-                    "pub"
-                } else {
-                    ""
-                };
                 let mut extern_item = mk_linkage(true, &new_name, ident)
                     .span(span)
-                    .set_mutbl(mutbl)
-                    .vis(visibility);
+                    .set_mutbl(mutbl);
+
+                // When putting extern statics into submodules, they need to be public to be accessible
+                if self.tcfg.reorganize_definitions {
+                    extern_item = extern_item.pub_();
+                };
+
                 if has_thread_duration {
                     extern_item = extern_item.single_attr("thread_local");
                 }
@@ -2407,16 +2406,13 @@ impl<'c> Translation<'c> {
                 ))
             } else {
                 // Translating an extern function declaration
+                let mut mk_ = mk_linkage(true, new_name, name).span(span);
 
                 // When putting extern fns into submodules, they need to be public to be accessible
-                let visibility = if self.tcfg.reorganize_definitions {
-                    "pub"
-                } else {
-                    ""
+                if self.tcfg.reorganize_definitions {
+                    mk_ = mk_.pub_();
                 };
 
-                let mut mk_ = mk_linkage(true, new_name, name).span(span).vis(visibility);
-
                 for attr in attrs {
                     mk_ = match attr {
                         c_ast::Attribute::Alias(aliasee) => mk_.str_attr("link_name", aliasee),
@@ -2703,7 +2699,7 @@ impl<'c> Translation<'c> {
 
                 if self.ast_context.is_va_list(typ.ctype) {
                     // translate `va_list` variables to `VaListImpl`s and omit the initializer.
-                    let pat_mut = mk().set_mutbl("mut").ident_pat(rust_name);
+                    let pat_mut = mk().mutbl().ident_pat(rust_name);
                     let ty = {
                         let path = vec!["core", "ffi", "VaListImpl"];
                         mk().path_ty(mk().abs_path(path))
@@ -2747,7 +2743,7 @@ impl<'c> Translation<'c> {
                     zeroed.to_pure_expr()
                 }
                 .expect("Expected decl initializer to not have any statements");
-                let pat_mut = mk().set_mutbl("mut").ident_pat(rust_name.clone());
+                let pat_mut = mk().mutbl().ident_pat(rust_name.clone());
                 let local_mut = mk().local(pat_mut, Some(ty.clone()), Some(zeroed));
                 if has_self_reference {
                     let assign = mk().assign_expr(mk().ident_expr(rust_name), init);
