transpile: separate top-level decl converting and emitting (#1371)

Right now, we sort the top-level decls once by their source order
(although we do this largely backwards, a separate issue) before we
convert them and then emit them (insert in the code). This means that
changing the conversion order also changes the order that they're
emitted in, which we don't want to do, so this separates the two steps.

Author: kkysen

c2rust-transpile/src/c_ast/mod.rs

@@ -986,8 +986,10 @@ impl TypedAstContext {
         }
     }
 
-    pub fn sort_top_decls(&mut self) {
-        // Group and sort declarations by file and by position
+    /// Sort the top-level declarations by file and source location
+    /// so that we preserve the ordering of all declarations in each file.
+    /// This preserves the order when we emit the converted declarations.
+    pub fn sort_top_decls_for_emitting(&mut self) {
         let mut decls_top = mem::take(&mut self.c_decls_top);
         decls_top.sort_unstable_by(|a, b| {
             let a = self.index(*a);

c2rust-transpile/src/translator/mod.rs

@@ -495,10 +495,6 @@ pub fn translate(
     };
 
     {
-        // Sort the top-level declarations by file and source location so that we
-        // preserve the ordering of all declarations in each file.
-        t.ast_context.sort_top_decls();
-
         t.locate_comments();
 
         // Headers often pull in declarations that are unused;
@@ -688,26 +684,37 @@ pub fn translate(
             }
         }
 
-        // Export top-level value declarations
-        for top_id in &t.ast_context.c_decls_top {
-            use CDeclKind::*;
-            let needs_export = match t.ast_context[*top_id].kind {
-                Function { is_implicit, .. } => !is_implicit,
-                Variable { .. } => true,
-                MacroObject { .. } => true, // Depends on `tcfg.translate_const_macros`, but handled in `fn convert_const_macro_expansion`.
-                MacroFunction { .. } => true, // Depends on `tcfg.translate_fn_macros`, but handled in `fn convert_fn_macro_invocation`.
-                _ => false,
-            };
-            if needs_export {
-                let decl = t.ast_context.get_decl(top_id).unwrap();
+        // Export top-level value declarations.
+        // We do this in a conversion pass and then an insertion pass
+        // so that the conversion order can differ from the order they're emitted in.
+        let mut converted_decls = t
+            .ast_context
+            .c_decls_top
+            .iter()
+            .filter_map(|&top_id| {
+                use CDeclKind::*;
+                let needs_export = match t.ast_context[top_id].kind {
+                    Function { is_implicit, .. } => !is_implicit,
+                    Variable { .. } => true,
+                    MacroObject { .. } => true, // Depends on `tcfg.translate_const_macros`, but handled in `fn convert_const_macro_expansion`.
+                    MacroFunction { .. } => true, // Depends on `tcfg.translate_fn_macros`, but handled in `fn convert_fn_macro_invocation`.
+                    _ => false,
+                };
+                if !needs_export {
+                    return None;
+                }
+
+                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
                     && decl_file_id.map_or(false, |id| id != t.main_file)
                 {
                     t.cur_file.set(decl_file_id);
                 }
-                match t.convert_decl(ctx, *top_id) {
+
+                // TODO use `.inspect_err` once stabilized in Rust 1.76.
+                let converted = match t.convert_decl(ctx, top_id) {
                     Err(e) => {
                         let decl_identifier = decl.kind.get_name().map_or_else(
                             || {
@@ -719,32 +726,47 @@ pub fn translate(
                         );
                         let msg = format!("Failed to translate {}: {}", decl_identifier, e);
                         translate_failure(t.tcfg, &msg);
+                        Err(e)
                     }
-                    Ok(converted_decl) => {
-                        use ConvertedDecl::*;
-                        match converted_decl {
-                            Item(item) => {
-                                t.insert_item(item, decl);
-                            }
-                            ForeignItem(item) => {
-                                t.insert_foreign_item(*item, decl);
-                            }
-                            Items(items) => {
-                                for item in items {
-                                    t.insert_item(item, decl);
-                                }
-                            }
-                            NoItem => {}
-                        }
-                    }
+                    Ok(converted) => Ok(converted),
                 }
+                .ok()?;
+
                 t.cur_file.take();
 
-                if t.tcfg.reorganize_definitions
-                    && decl_file_id.map_or(false, |id| id != t.main_file)
-                {
-                    t.generate_submodule_imports(*top_id, decl_file_id);
+                Some((top_id, converted))
+            })
+            .collect::<HashMap<_, _>>();
+
+        t.ast_context.sort_top_decls_for_emitting();
+
+        for top_id in &t.ast_context.c_decls_top {
+            let decl = t.ast_context.get_decl(top_id).unwrap();
+            let decl_file_id = t.ast_context.file_id(decl);
+            // TODO use `let else` when it stabilizes.
+            let converted_decl = match converted_decls.remove(top_id) {
+                Some(converted) => converted,
+                None => continue,
+            };
+
+            use ConvertedDecl::*;
+            match converted_decl {
+                Item(item) => {
+                    t.insert_item(item, decl);
                 }
+                ForeignItem(item) => {
+                    t.insert_foreign_item(*item, decl);
+                }
+                Items(items) => {
+                    for item in items {
+                        t.insert_item(item, decl);
+                    }
+                }
+                NoItem => {}
+            };
+
+            if t.tcfg.reorganize_definitions && decl_file_id.map_or(false, |id| id != t.main_file) {
+                t.generate_submodule_imports(*top_id, decl_file_id);
             }
         }