transpile: replace --reorganize-definitions/--disable-refactoring with --emit-refactor-annotations/--refactor

README.md

@@ -223,7 +223,7 @@ normal Rust modules.
 They will export and import functions through the C API.
 These modules can be compiled together into a single static Rust library or binary.
 
-You can run with `--reorganize-definitions` (which invokes `c2rust-refactor`),
+You can run with `--refactor`,
 which should deduplicate definitions and directly import them
 with `use`s instead of through the C API.
 

c2rust-refactor/src/transform/reorganize_definitions.rs

@@ -39,7 +39,7 @@ use super::externs;
 /// Usage: `reorganize_definitions`
 ///
 /// This refactoring operates on code transpiled with the
-/// `--reorganize-definitions` flag.
+/// `--emit-refactor-annotations` flag.
 ///
 /// This pass refactors a crate to de-duplicate declarations, move them into
 /// their relevant modules and import the items as needed, rather than using

c2rust-transpile/src/lib.rs

@@ -96,19 +96,24 @@ pub struct TranspilerConfig {
     pub translate_valist: bool,
     pub overwrite_existing: bool,
     pub reduce_type_annotations: bool,
-    pub reorganize_definitions: bool,
     pub enabled_warnings: HashSet<Diagnostic>,
     pub emit_no_std: bool,
     pub output_dir: Option<PathBuf>,
     pub translate_const_macros: TranslateMacros,
     pub translate_fn_macros: TranslateMacros,
     pub disable_rustfmt: bool,
-    pub disable_refactoring: bool,
     pub preserve_unused_functions: bool,
     pub log_level: log::LevelFilter,
     pub edition: RustEdition,
 
-    /// Run `c2rust-postprocess` after transpiling and potentially refactoring.
+    /// Run `c2rust-refactor` after transpiling (and before [`Self::postprocess`]).
+    pub refactor: bool,
+
+    /// Emit annotations needed by the refactorer for certain transforms, such as `reorganize_definitions`.
+    /// Implied by [`Self::refactor`].
+    pub emit_refactor_annotations: bool,
+
+    /// Run `c2rust-postprocess` after transpiling (and after [`Self::refactor`]).
     pub postprocess: bool,
 
     // Options that control build files
@@ -547,7 +552,7 @@ fn reorganize_definitions(
     crate_file: Option<&Path>,
 ) -> Result<(), Error> {
     // We only run the reorganization refactoring if we emitted a fresh crate file
-    if crate_file.is_none() || tcfg.disable_refactoring || !tcfg.reorganize_definitions {
+    if crate_file.is_none() || !tcfg.refactor {
         return Ok(());
     }
 

c2rust-transpile/src/translator/mod.rs

@@ -841,7 +841,7 @@ pub fn translate(
         {
             let export_type = |decl_id: CDeclId, decl: &CDecl| {
                 let decl_file_id = t.ast_context.file_id(decl);
-                if t.tcfg.reorganize_definitions {
+                if t.tcfg.emit_refactor_annotations {
                     t.cur_file.set(decl_file_id);
                 }
                 match t.convert_decl(ctx, decl_id) {
@@ -870,7 +870,7 @@ pub fn translate(
                 }
                 t.cur_file.take();
 
-                if t.tcfg.reorganize_definitions
+                if t.tcfg.emit_refactor_annotations
                     && decl_file_id.map_or(false, |id| id != t.main_file)
                 {
                     let name = t
@@ -929,7 +929,7 @@ pub fn translate(
                 let decl = t.ast_context.get_decl(&top_id).unwrap();
                 let decl_file_id = t.ast_context.file_id(decl);
 
-                if t.tcfg.reorganize_definitions
+                if t.tcfg.emit_refactor_annotations
                     && decl_file_id.map_or(false, |id| id != t.main_file)
                 {
                     t.cur_file.set(decl_file_id);
@@ -991,7 +991,9 @@ pub fn translate(
                 NoItem => {}
             };
 
-            if t.tcfg.reorganize_definitions && decl_file_id.map_or(false, |id| id != t.main_file) {
+            if t.tcfg.emit_refactor_annotations
+                && decl_file_id.map_or(false, |id| id != t.main_file)
+            {
                 log::debug!("emitting any imports needed by {:?}", decl.kind.get_name());
                 t.generate_submodule_imports(*top_id, decl_file_id);
             }
@@ -1025,7 +1027,7 @@ pub fn translate(
         // Header Reorganization: Submodule Item Stores
         for (file_id, ref mut mod_item_store) in t.items.borrow_mut().iter_mut() {
             if *file_id != t.main_file {
-                if tcfg.reorganize_definitions {
+                if tcfg.emit_refactor_annotations {
                     t.use_feature("register_tool");
                 }
                 let mut submodule = make_submodule(
@@ -1034,7 +1036,7 @@ pub fn translate(
                     *file_id,
                     &mut new_uses,
                     &t.mod_names,
-                    tcfg.reorganize_definitions,
+                    tcfg.emit_refactor_annotations,
                     tcfg.edition,
                 );
                 let comments = t.comment_context.get_remaining_comments(*file_id);
@@ -2110,7 +2112,7 @@ impl<'c> Translation<'c> {
                     .set_mutbl(mutbl);
 
                 // When putting extern statics into submodules, they need to be public to be accessible
-                if self.tcfg.reorganize_definitions {
+                if self.tcfg.emit_refactor_annotations {
                     extern_item = extern_item.pub_();
                 };
 
@@ -2597,7 +2599,7 @@ impl<'c> Translation<'c> {
                 let mut mk_ = mk_linkage(true, new_name, name, self.tcfg.edition).span(span);
 
                 // When putting extern fns into submodules, they need to be public to be accessible
-                if self.tcfg.reorganize_definitions {
+                if self.tcfg.emit_refactor_annotations {
                     mk_ = mk_.pub_();
                 };
 
@@ -3584,7 +3586,7 @@ impl<'c> Translation<'c> {
                     .ok_or_else(|| format_err!("name not declared: '{}'", varname))?;
 
                 // Import the referenced global decl into our submodule
-                if self.tcfg.reorganize_definitions {
+                if self.tcfg.emit_refactor_annotations {
                     self.add_import(decl_id, &rustname);
                     // match decl {
                     //     CDeclKind::Variable { is_defn: false, .. } => {}
@@ -5009,7 +5011,7 @@ impl<'c> Translation<'c> {
     fn insert_item(&self, mut item: Box<Item>, decl: &CDecl) {
         let decl_file_id = self.ast_context.file_id(decl);
 
-        if self.tcfg.reorganize_definitions {
+        if self.tcfg.emit_refactor_annotations {
             self.use_feature("register_tool");
             let attrs = item_attrs(&mut item).expect("no attrs field on unexpected item variant");
             add_src_loc_attr(attrs, &decl.loc.as_ref().map(|x| x.begin()));
@@ -5027,7 +5029,7 @@ impl<'c> Translation<'c> {
     fn insert_foreign_item(&self, mut item: ForeignItem, decl: &CDecl) {
         let decl_file_id = self.ast_context.file_id(decl);
 
-        if self.tcfg.reorganize_definitions {
+        if self.tcfg.emit_refactor_annotations {
             self.use_feature("register_tool");
             let attrs = foreign_item_attrs(&mut item)
                 .expect("no attrs field on unexpected foreign item variant");

c2rust-transpile/tests/snapshots.rs

@@ -45,17 +45,17 @@ fn config(edition: RustEdition) -> TranspilerConfig {
         translate_valist: true,
         overwrite_existing: true,
         reduce_type_annotations: false,
-        reorganize_definitions: false,
         enabled_warnings: Default::default(),
         emit_no_std: false,
         output_dir: None,
         translate_const_macros: Default::default(),
         translate_fn_macros: Default::default(),
         disable_rustfmt: false,
-        disable_refactoring: false,
         preserve_unused_functions: false,
         log_level: log::LevelFilter::Warn,
         edition,
+        refactor: false,
+        emit_refactor_annotations: false,
         postprocess: false,
         emit_build_files: false,
         binaries: Vec::new(),

c2rust/src/bin/c2rust-transpile.rs

@@ -141,9 +141,14 @@ struct Args {
     #[clap(long)]
     reduce_type_annotations: bool,
 
-    /// Output file in such a way that the refactoring tool can deduplicate code
-    #[clap(short = 'r', long)]
-    reorganize_definitions: bool,
+    /// Run `c2rust-refactor` after transpiling (and before `--postprocess`).
+    #[clap(long)]
+    refactor: bool,
+
+    /// Emit annotations needed by the refactorer for certain transforms, such as `reorganize_definitions`.
+    /// Implied by `--refactor`.
+    #[clap(long)]
+    emit_refactor_annotations: bool,
 
     /// Run `c2rust-postprocess` after transpiling and potentially refactoring.
     ///
@@ -291,15 +296,15 @@ fn main() {
         translate_const_macros: args.translate_const_macros.into(),
         translate_fn_macros: args.translate_fn_macros.into(),
         disable_rustfmt: args.disable_rustfmt,
-        disable_refactoring: args.disable_refactoring,
         preserve_unused_functions: args.preserve_unused_functions,
 
         use_c_loop_info: !args.ignore_c_loop_info,
         use_c_multiple_info: !args.ignore_c_multiple_info,
         simplify_structures: !args.no_simplify_structures,
         overwrite_existing: args.overwrite_existing,
         reduce_type_annotations: args.reduce_type_annotations,
-        reorganize_definitions: args.reorganize_definitions,
+        refactor: args.refactor,
+        emit_refactor_annotations: args.emit_refactor_annotations || args.refactor,
         postprocess: args.remote_llm_postprocess,
         emit_modules: args.emit_modules,
         emit_build_files: args.emit_build_files,

scripts/common.py

@@ -475,7 +475,7 @@ def transpile(cc_db_path: str,
               use_fakechecks: bool = False,
               cross_check_config: List[str] = [],
               incremental_relooper: bool = True,
-              reorganize_definitions: bool = False) -> bool:
+              refactor: bool = False) -> bool:
     """
     run the transpiler on all C files in a compile commands database.
     """
@@ -502,8 +502,8 @@ def transpile(cc_db_path: str,
             args.append(ccc)
     if not incremental_relooper:
         args.append('--no-incremental-relooper')
-    if reorganize_definitions:
-        args.append('--reorganize-definitions')
+    if refactor:
+        args.append('--refactor')
     if filter:
         args.append('--filter')
         args.append(filter)

scripts/test_translator.py

@@ -74,7 +74,7 @@ def __init__(self, log_level: str, path: str, flags: Set[str] = set()) -> None:
         self.disable_incremental_relooper = "disable_incremental_relooper" in flags
         self.disallow_current_block = "disallow_current_block" in flags
         self.translate_const_macros = "translate_const_macros" in flags
-        self.reorganize_definitions = "reorganize_definitions" in flags
+        self.refactor = "refactor" in flags
         self.emit_build_files = "emit_build_files" in flags
 
     def translate(self, cc_db: str, edition: RustEdition, ld_lib_path: str, extra_args: List[str] = []) -> RustFile:
@@ -102,8 +102,8 @@ def translate(self, cc_db: str, edition: RustEdition, ld_lib_path: str, extra_ar
         if self.translate_const_macros:
             args.append("--translate-const-macros")
             args.append("experimental")
-        if self.reorganize_definitions:
-            args.append("--reorganize-definitions")
+        if self.refactor:
+            args.append("--refactor")
         if self.emit_build_files:
             args.append("--emit-build-files")
 

tests/integration/tests/curl/conf.yml

@@ -12,7 +12,7 @@ requirements:
 
 transpile:
   autogen: true
-  # tflags: --reorganize-definitions
+  # tflags: --emit-refactor-annotations
   binary: tool_main
 
 cargo.transpile:

tests/integration/tests/json-c/conf.yml

@@ -6,7 +6,7 @@ requirements:
 
 transpile:
   autogen: true
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
 
 cargo.transpile:
   autogen: true

tests/integration/tests/libxml2/conf.yml

@@ -1,7 +1,7 @@
 transpile:
   autogen: true
   binary: runtest
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
 
 cargo.transpile:
   autogen: true

tests/integration/tests/lua/conf.yml

@@ -7,7 +7,7 @@ requirements:
 
 transpile:
   autogen: true
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
   binary: lua
 
 cargo.transpile:

tests/integration/tests/nginx/conf.yml

@@ -9,7 +9,7 @@ requirements:
 transpile:
   autogen: true
   # blocked on https://github.com/immunant/c2rust/issues/266
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
   binary: nginx
 
 cargo.transpile:

tests/integration/tests/python2/conf.yml

@@ -17,7 +17,7 @@ requirements:
 
 transpile:
   autogen: true
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
   binary: python
 
 cargo.transpile:

tests/integration/tests/ruby/conf.yml

@@ -9,7 +9,7 @@ requirements:
 
 transpile:
   autogen: true
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
   binary: ruby
   # cflags: "-O0"
 

tests/integration/tests/snudown/conf.yml

@@ -7,7 +7,7 @@ requirements:
 
 transpile:
   autogen: true
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
 
 cargo.transpile:
   autogen: true

tests/integration/tests/zstd/conf.yml

@@ -8,7 +8,7 @@ transpile:
   autogen: true
   binary: fullbench # zstdcli
   cflags: -Qunused-arguments
-  tflags: --reorganize-definitions --disable-refactoring
+  tflags: --emit-refactor-annotations
 
 cargo.transpile:
   autogen: true

tests/unit/comments/src/comments.c

@@ -1,4 +1,4 @@
-//! translate_const_macros, reorganize_definitions, emit_build_files
+//! translate_const_macros, refactor, emit_build_files
 /* top level doc comment
  * second line
  */

tests/unit/items/src/test_fn_attrs.rs

@@ -6,8 +6,8 @@ pub fn test_fn_attrs() {
     // so instead we're checking the source itself
     let src = include_str!("fn_attrs.rs");
 
-    // Remove the c2rust::src_loc annotation, which is only produced if
-    // --reorganize-definitions is enabled.
+    // Remove the `c2rust::src_loc` annotation, which is only produced if
+    // `--emit-refactor-annotations` is enabled.
     let mut lines = src.lines().collect::<Vec<_>>();
     lines.retain(|x| !x.contains("#[c2rust::src_loc"));
     let src = lines.join("\n");

tests/unit/modules/src/modules.c

@@ -1,4 +1,4 @@
-//! translate_const_macros, reorganize_definitions
+//! translate_const_macros, refactor
 
 #include <stddef.h>
 #include "other_mod2.h"

tests/unit/statics/src/test_sections.rs

@@ -40,8 +40,8 @@ pub fn test_sectioned_used_static() {
 
         let mut lines = src.lines().collect::<Vec<_>>();
 
-        // Remove the c2rust::src_loc annotation, which is only produced if
-        // --reorganize-definitions is enabled.
+        // Remove the `c2rust::src_loc` annotation, which is only produced if
+        // `--emit-refactor-annotations` is enabled.
         lines.retain(|x| !x.contains("#[c2rust::src_loc"));
         let src = lines.join("\n");
 
@@ -65,7 +65,7 @@ pub static mut rust_initialized_extern: ::core::ffi::c_int = 1 as ::core::ffi::c
 "#
             .trim(),
         ));
-        // This static is pub only with --reorganize-definitions
+        // This static is pub only with `--emit-refactor-annotations`.
         let aliased_static_syntax = |public| {
             format!(
                 r#"