From 6b9758f9a909d69fdf5d6f98976fcd6815c230d6 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Mon, 9 Feb 2026 14:46:57 -0800 Subject: [PATCH 01/13] postprocess: trim functions before attempting comment transfer The C declarations can contain preprocessor definitions with their own associated comments. Sometimes we also get license headers and similar front matter. Use a separate transformation to remove this front matter such that validation of comment insertion only take the relevant comments into account. Note: the trim step is currently tightly integrated into the comment transfer step but I expect we'll want to run it before other transforms as well. This can be handled in a follow-up change. --- .../postprocess/transforms/base.py | 2 +- .../postprocess/transforms/comments.py | 30 +++++++ .../postprocess/transforms/trim.py | 88 +++++++++++++++++++ c2rust-postprocess/tests/test_definitions.py | 12 +-- 4 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 c2rust-postprocess/postprocess/transforms/trim.py diff --git a/c2rust-postprocess/postprocess/transforms/base.py b/c2rust-postprocess/postprocess/transforms/base.py index dc2c15c6d4..6a3c112be8 100644 --- a/c2rust-postprocess/postprocess/transforms/base.py +++ b/c2rust-postprocess/postprocess/transforms/base.py @@ -36,7 +36,7 @@ def apply_ident( c_definition: CDefinition, identifier: str, update_rust: bool = True, - ) -> None: + ) -> str | None: """ Implementations should apply transform to a single Rust definition with the given identifier. diff --git a/c2rust-postprocess/postprocess/transforms/comments.py b/c2rust-postprocess/postprocess/transforms/comments.py index e25be46b43..835f17842b 100644 --- a/c2rust-postprocess/postprocess/transforms/comments.py +++ b/c2rust-postprocess/postprocess/transforms/comments.py @@ -13,6 +13,7 @@ ) from postprocess.models import AbstractGenerativeModel, api_key_from_env from postprocess.transforms.base import AbstractTransform, TransformError +from postprocess.transforms.trim import TrimTransform from postprocess.utils import get_highlighted_rust, remove_backticks # TODO: get from model @@ -77,6 +78,7 @@ def __init__(self, cache: AbstractCache, model: AbstractGenerativeModel): super().__init__(SYSTEM_INSTRUCTION) self.cache = cache self.model = model + self.trim_transform = TrimTransform(cache, model) def apply_ident( self, @@ -100,6 +102,34 @@ def apply_ident( logging.info(f"Skipping C function without comments: {identifier}") return + match self.trim_transform.apply_ident( + rust_source_file=rust_source_file, + rust_definition=rust_definition, + c_definition=c_definition, + identifier=identifier, + update_rust=False, # nothing to update here + ): + case None: + logging.info( + f"Trim transform produced no trimmed C definition for " + f"{identifier}; continuing with the original definition" + ) + case str() as trimmed_c_definition: + # TODO: consider trimming both the definition and the preprocessed + # definition instead of possibly replacing the original + # definition with the trimmed and preprocessed one. + c_definition = CDefinition( + definition=trimmed_c_definition, preprocessed_definition=None + ) + c_comments = _get_transferable_c_comments(c_definition) + if not c_comments: + logging.info(f"Skipping C function without comments: {identifier}") + return + case _: + raise AssertionError( + "Unexpected return type from trim transform: expected None or str" + ) + # TODO: make this function take a model and get prompt from model prompt_text = """ Transfer the comments from the following C function to the corresponding Rust function. diff --git a/c2rust-postprocess/postprocess/transforms/trim.py b/c2rust-postprocess/postprocess/transforms/trim.py new file mode 100644 index 0000000000..96ab787d80 --- /dev/null +++ b/c2rust-postprocess/postprocess/transforms/trim.py @@ -0,0 +1,88 @@ +import logging +from pathlib import Path +from textwrap import dedent + +from postprocess.cache import AbstractCache +from postprocess.definitions import CDefinition, get_c_comments +from postprocess.models import AbstractGenerativeModel +from postprocess.transforms.base import AbstractTransform +from postprocess.utils import remove_backticks + +SYSTEM_INSTRUCTION = ( + "You are a helpful assistant that removes top-of-file prologues and" + " other unnecessary content such as preprocessor definitions and" + " comments that preceede and are unrelated to the definition of a C function." +) + + +class TrimTransform(AbstractTransform): + def __init__(self, cache: AbstractCache, model: AbstractGenerativeModel): + super().__init__(SYSTEM_INSTRUCTION) + self.cache = cache + self.model = model + + def apply_ident( + self, + rust_source_file: Path, + rust_definition: str, + c_definition: CDefinition, + identifier: str, + update_rust: bool = True, + ) -> str | None: + c_comments = get_c_comments(c_definition.effective) + if not c_comments: + logging.info( + f"{self.__class__.__name__}: " + f"Skipping C function without comments: {identifier}" + ) + return + + prompt = """ + Remove any prologues, preprocessor definitions, and comments that are unrelated to the definition + of the C function `{identifier}`. Respond with the trimmed C function definition; say nothing else. + + C function: + ```c + {c_definition} + ``` + """ # noqa: E501 + prompt = dedent( + prompt + ).strip() # note: dedent then format since the C definition isn't indented + prompt = prompt.format( + identifier=identifier, c_definition=c_definition.effective + ) + + messages = [ + {"role": "user", "content": prompt}, + ] + + transform = self.__class__.__name__ + model = self.model.id + if response := self.cache.lookup( + transform=transform, + identifier=identifier, + model=model, + messages=messages, + ): + return response + + response = self.model.generate_with_tools(messages) + + if response is None: + logging.error("Model returned no response") + return response + + # TODO: validate that function definition is still present? + + response = remove_backticks(response) + + self.cache.update( + transform=transform, + identifier=identifier, + model=model, + messages=messages, + response=response, + ) + + return response diff --git a/c2rust-postprocess/tests/test_definitions.py b/c2rust-postprocess/tests/test_definitions.py index 590b490bc1..ef754288a3 100644 --- a/c2rust-postprocess/tests/test_definitions.py +++ b/c2rust-postprocess/tests/test_definitions.py @@ -43,16 +43,16 @@ def test_get_rust_function_spans(transpile_qsort, pytestconfig): { "name": "partition", "start_line": 17, - "end_line": 40, + "end_line": 37, "start_byte": 362, - "end_byte": 1225, + "end_byte": 1070, }, { "name": "quickSort", - "start_line": 42, - "end_line": 52, - "start_byte": 1239, - "end_byte": 1598, + "start_line": 39, + "end_line": 49, + "start_byte": 1084, + "end_byte": 1443, }, ] From 915cbb67c086ad4c5849101e9a9bc6f95c925193 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Thu, 11 Jun 2026 00:31:32 -0700 Subject: [PATCH 02/13] postprocess: improve logging of skipped and transformed functions --- c2rust-postprocess/postprocess/transforms/comments.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/c2rust-postprocess/postprocess/transforms/comments.py b/c2rust-postprocess/postprocess/transforms/comments.py index 835f17842b..98ea7070b6 100644 --- a/c2rust-postprocess/postprocess/transforms/comments.py +++ b/c2rust-postprocess/postprocess/transforms/comments.py @@ -92,8 +92,7 @@ def apply_ident( if rust_comments: logging.info( f"Skipping Rust fn {identifier} with existing comments:\ - \n{rust_comments} in\ - \n{rust_definition}" + \n{get_highlighted_rust(rust_definition)}" ) return @@ -196,7 +195,10 @@ def apply_ident( response=response, ) - print(get_highlighted_rust(rust_fn)) + logging.info( + f"Comments transferred to Rust fn {identifier}:\ + \n{get_highlighted_rust(rust_fn)}" + ) # TODO: move this to apply_file? # the challenge is that not all transforms will update Rust code From 14ca847e087633e097abebec7aab121541f8848c Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Tue, 7 Jul 2026 20:51:31 -0700 Subject: [PATCH 03/13] postprocess: don't extract include paths as comments --- c2rust-postprocess/postprocess/definitions/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/c2rust-postprocess/postprocess/definitions/__init__.py b/c2rust-postprocess/postprocess/definitions/__init__.py index 4521c75be3..f54510eadb 100644 --- a/c2rust-postprocess/postprocess/definitions/__init__.py +++ b/c2rust-postprocess/postprocess/definitions/__init__.py @@ -206,10 +206,18 @@ def walk(node: Node) -> Generator[str]: return get_comments_text(walk(tree.root_node)) +def is_comment_token(tok_type: Any) -> bool: + """True for real comments; excludes directives and include paths.""" + return tok_type in Comment and tok_type not in { + Comment.Preproc, + Comment.PreprocFile, + } + + def get_comments(code: str, lexer: RegexLexer) -> list[str]: comments = [] for tok_type, tok_value in lex(code, lexer): - if tok_type in Comment and tok_type != Comment.Preproc: + if is_comment_token(tok_type): # Keep exactly what appears, including delimiters (//, /* */) comments.append(tok_value) return comments From 84184956bd90d0807afc41e881ff9c8b7763a09c Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Tue, 7 Jul 2026 20:51:36 -0700 Subject: [PATCH 04/13] transpile: emit decl_line in C decl map --- c2rust-transpile/src/translator/mod.rs | 24 ++++++++++++------- .../snapshots__c_decls@directives.c.snap | 3 +++ .../snapshots__c_decls@line_directives.c.snap | 1 + .../snapshots/snapshots__c_decls@nh.c.snap | 18 ++++++++++++++ 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/c2rust-transpile/src/translator/mod.rs b/c2rust-transpile/src/translator/mod.rs index c355090fda..82d956693e 100644 --- a/c2rust-transpile/src/translator/mod.rs +++ b/c2rust-transpile/src/translator/mod.rs @@ -593,7 +593,8 @@ pub fn emit_c_decl_map( let byte_offset_of = |loc| src_loc_to_byte_offset(&line_end_offsets, loc); // Slice into the source file, fixing up the ends to account for Clang AST quirks. - let slice_decl_with_fixups = |begin: SrcLoc, mid: SrcLoc, end: SrcLoc| -> &[u8] { + // Also returns the line within the slice where the decl itself (`mid`) begins. + let slice_decl_with_fixups = |begin: SrcLoc, mid: SrcLoc, end: SrcLoc| -> (&[u8], usize) { assert!(begin.line <= mid.line, "{} <= {}", begin.line, mid.line); assert!(mid.line <= end.line, "{} <= {}", mid.line, end.line); @@ -656,23 +657,28 @@ pub fn emit_c_decl_map( end_offset += 1; } - &file_content[begin_offset..end_offset] + // Line within the emitted text where the decl itself begins; earlier + // lines hold preceding comments and other front matter. + let decl_line = file_content[begin_offset..mid_offset.max(begin_offset)] + .iter() + .filter(|&&c| c == b'\n') + .count(); + + (&file_content[begin_offset..end_offset], decl_line) }; let definitions = path_to_c_source_range .into_iter() .map(|(ident, (decl_id, range))| { let path = ident.to_string(); - let c_src = std::str::from_utf8(slice_decl_with_fixups( - range.earliest_begin, - range.strict_begin, - range.end, - )) - .unwrap(); + let (c_src, decl_line) = + slice_decl_with_fixups(range.earliest_begin, range.strict_begin, range.end); + let c_src = std::str::from_utf8(c_src).unwrap(); ( path, CDeclMapDefinition { definition: c_src.to_owned(), + decl_line, preprocessed_definition: preprocessed_definitions.get(&decl_id).cloned(), }, ) @@ -833,6 +839,8 @@ pub struct DeclMap { #[derive(Serialize)] struct CDeclMapDefinition { definition: String, + /// 0-based line within `definition` where the decl itself begins. + decl_line: usize, preprocessed_definition: Option, } diff --git a/c2rust-transpile/tests/snapshots/snapshots__c_decls@directives.c.snap b/c2rust-transpile/tests/snapshots/snapshots__c_decls@directives.c.snap index c8de136963..7067a2127a 100644 --- a/c2rust-transpile/tests/snapshots/snapshots__c_decls@directives.c.snap +++ b/c2rust-transpile/tests/snapshots/snapshots__c_decls@directives.c.snap @@ -5,14 +5,17 @@ expression: cat tests/c_decls_snapshots/directives.c_decls.json { "definitions": { "cond_fn": { + "decl_line": 1, "definition": "// comment for cond_fn\nint cond_fn(void) {\n#ifdef NOT_DEFINED\n /* comment in inactive region; must not appear in preprocessed text */\n return 1;\n#else\n // comment in active region\n return 2;\n#endif\n}", "preprocessed_definition": "// comment for cond_fn\nint cond_fn(void) {\n\n\n\n\n // comment in active region\n return 2;\n\n}" }, "shadowed_name": { + "decl_line": 0, "definition": "struct shadowed_name *shadowed_name(void) {\n return 0;\n}", "preprocessed_definition": "struct shadowed_name *shadowed_name(void) {\n return 0;\n}" }, "uses_macro": { + "decl_line": 1, "definition": "/* comment for uses_macro */\nint uses_macro(int n) {\n /* macro invocation should stay unexpanded */\n return DOUBLE(n);\n}", "preprocessed_definition": "/* comment for uses_macro */\nint uses_macro(int n) {\n /* macro invocation should stay unexpanded */\n return DOUBLE(n);\n}" } diff --git a/c2rust-transpile/tests/snapshots/snapshots__c_decls@line_directives.c.snap b/c2rust-transpile/tests/snapshots/snapshots__c_decls@line_directives.c.snap index 3e68a59ba9..68ca46262d 100644 --- a/c2rust-transpile/tests/snapshots/snapshots__c_decls@line_directives.c.snap +++ b/c2rust-transpile/tests/snapshots/snapshots__c_decls@line_directives.c.snap @@ -5,6 +5,7 @@ expression: cat tests/c_decls_snapshots/line_directives.c_decls.json { "definitions": { "before_line": { + "decl_line": 0, "definition": "int before_line(void) {\n return 1;\n}", "preprocessed_definition": null } diff --git a/c2rust-transpile/tests/snapshots/snapshots__c_decls@nh.c.snap b/c2rust-transpile/tests/snapshots/snapshots__c_decls@nh.c.snap index ded721fdfa..d96ed84b6b 100644 --- a/c2rust-transpile/tests/snapshots/snapshots__c_decls@nh.c.snap +++ b/c2rust-transpile/tests/snapshots/snapshots__c_decls@nh.c.snap @@ -5,74 +5,92 @@ expression: cat tests/c_decls_snapshots/nh.c_decls.json { "definitions": { "FOO": { + "decl_line": 0, "definition": "FOO 4000+50", "preprocessed_definition": null }, "a": { + "decl_line": 2, "definition": "#include \n#include \nint a;", "preprocessed_definition": null }, "a1": { + "decl_line": 1, "definition": "// comment before multiple decl\nint a1,", "preprocessed_definition": null }, "a2": { + "decl_line": 0, "definition": ", a2,", "preprocessed_definition": null }, "a3": { + "decl_line": 0, "definition": ", a3;", "preprocessed_definition": null }, "another": { + "decl_line": 0, "definition": "int another;", "preprocessed_definition": null }, "another_func": { + "decl_line": 0, "definition": "int\nanother_func(void)\n{\n int var = 0;\n return 4 + var;\n}", "preprocessed_definition": "int\nanother_func(void)\n{\n int var = 0;\n return 4 + var;\n}" }, "bb": { + "decl_line": 0, "definition": "int bb;", "preprocessed_definition": null }, "ccc": { + "decl_line": 0, "definition": "/*comment for cc*/int ccc;", "preprocessed_definition": null }, "d": { + "decl_line": 0, "definition": "int d;", "preprocessed_definition": null }, "e": { + "decl_line": 0, "definition": "int e;", "preprocessed_definition": null }, "f": { + "decl_line": 0, "definition": "void f(void){}", "preprocessed_definition": null }, "forward_decl_before_other_def": { + "decl_line": 1, "definition": "/*real defn*/\nvoid forward_decl_before_other_def(int x) { }", "preprocessed_definition": "/*real defn*/\nvoid forward_decl_before_other_def(int x) { }" }, "func_after_decl": { + "decl_line": 0, "definition": "void func_after_decl(int n) {}", "preprocessed_definition": "void func_after_decl(int n) {}" }, "func_holding_define": { + "decl_line": 0, "definition": "int\nfunc_holding_define(void)\n{\n#define FOO 4000+50\n return FOO;\n}", "preprocessed_definition": "int\nfunc_holding_define(void)\n{\n#define FOO 4000+50\n return FOO;\n}" }, "g": { + "decl_line": 0, "definition": "void g(void){}", "preprocessed_definition": null }, "h": { + "decl_line": 0, "definition": "/*comment for h*/void h(void){}", "preprocessed_definition": null }, "ngx_hash_init": { + "decl_line": 1, "definition": "//comment for ngx_hash_init\nvoid\nngx_hash_init(void)\n{\n size_t len;\n ngx_uint_t align = len & ~sizeof(void *);\n}", "preprocessed_definition": "//comment for ngx_hash_init\nvoid\nngx_hash_init(void)\n{\n size_t len;\n ngx_uint_t align = len & ~sizeof(void *);\n}" } From 2a8e3dc693e5019ecbdcb1fc636be442ce66c409 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Tue, 7 Jul 2026 20:53:54 -0700 Subject: [PATCH 05/13] postprocess: gate and validate the trim transform --- .../postprocess/definitions/__init__.py | 4 + .../postprocess/transforms/trim.py | 108 +++++++++++--- .../tests/test_comments_transform.py | 17 ++- .../tests/test_trim_transform.py | 141 ++++++++++++++++++ 4 files changed, 241 insertions(+), 29 deletions(-) create mode 100644 c2rust-postprocess/tests/test_trim_transform.py diff --git a/c2rust-postprocess/postprocess/definitions/__init__.py b/c2rust-postprocess/postprocess/definitions/__init__.py index f54510eadb..a1640e8096 100644 --- a/c2rust-postprocess/postprocess/definitions/__init__.py +++ b/c2rust-postprocess/postprocess/definitions/__init__.py @@ -246,6 +246,9 @@ def get_rust_definitions(root_rust_source_file: Path) -> dict[str, str]: class CDefinition: definition: str preprocessed_definition: str | None + # 0-based line within `definition` where the decl itself begins; earlier + # lines hold preceding comments and other front matter. + decl_line: int | None = None @property def effective(self) -> str: @@ -278,6 +281,7 @@ def get_c_definitions(root_rust_source_file: Path) -> dict[str, CDefinition]: identifier: CDefinition( definition=decl["definition"], preprocessed_definition=decl.get("preprocessed_definition"), + decl_line=decl.get("decl_line"), ) for identifier, decl in c_decls["definitions"].items() } diff --git a/c2rust-postprocess/postprocess/transforms/trim.py b/c2rust-postprocess/postprocess/transforms/trim.py index 96ab787d80..5c9467641e 100644 --- a/c2rust-postprocess/postprocess/transforms/trim.py +++ b/c2rust-postprocess/postprocess/transforms/trim.py @@ -2,8 +2,11 @@ from pathlib import Path from textwrap import dedent +from pygments import lex +from pygments.lexers.c_cpp import CLexer + from postprocess.cache import AbstractCache -from postprocess.definitions import CDefinition, get_c_comments +from postprocess.definitions import CDefinition, get_c_comments, is_comment_token from postprocess.models import AbstractGenerativeModel from postprocess.transforms.base import AbstractTransform from postprocess.utils import remove_backticks @@ -15,6 +18,45 @@ ) +def _scan_lines(code: str) -> list[tuple[bool, bool]]: + """Classify each line of C code as `(has_comment, has_code)`.""" + n_lines = len(code.splitlines()) + flags = [[False, False] for _ in range(n_lines)] + line = 0 + for tok_type, tok_value in lex(code, CLexer()): + for i, part in enumerate(tok_value.split("\n")): + if i: + line += 1 + if line >= n_lines or not part.strip(): + continue + if is_comment_token(tok_type): + flags[line][0] = True # has_comment + else: + flags[line][1] = True # has_code + return [(has_comment, has_code) for has_comment, has_code in flags] + + +def _prefix_needs_trim(definition: str, decl_line: int) -> bool: + """ + Whether the front matter before the decl contains comments beyond a single + comment block adjacent to the decl (i.e. beyond its doc comment). + """ + lines = _scan_lines("\n".join(definition.splitlines()[:decl_line])) + i = len(lines) - 1 + while i >= 0 and lines[i] == (False, False): + i -= 1 # blank lines between the doc comment and the decl + while i >= 0 and lines[i] == (True, False): + i -= 1 # the doc comment itself + return any(has_comment for has_comment, _ in lines[: i + 1]) + + +def _is_suffix_ignoring_whitespace(trimmed: str, original: str) -> bool: + squashed_trimmed = "".join(trimmed.split()) + return bool(squashed_trimmed) and "".join(original.split()).endswith( + squashed_trimmed + ) + + class TrimTransform(AbstractTransform): def __init__(self, cache: AbstractCache, model: AbstractGenerativeModel): super().__init__(SYSTEM_INSTRUCTION) @@ -29,13 +71,26 @@ def apply_ident( identifier: str, update_rust: bool = True, ) -> str | None: - c_comments = get_c_comments(c_definition.effective) - if not c_comments: + definition = c_definition.definition + + # Decide deterministically whether trimming is needed at all; only the + # judgment of which prefix comments belong to the function is left to + # the model. Without `decl_line` (older c_decls.json), always attempt. + if c_definition.decl_line is not None and not _prefix_needs_trim( + definition, c_definition.decl_line + ): + logging.info( + f"{self.__class__.__name__}: " + f"Skipping C function without front matter to trim: {identifier}" + ) + return None + + if not get_c_comments(definition): logging.info( f"{self.__class__.__name__}: " f"Skipping C function without comments: {identifier}" ) - return + return None prompt = """ Remove any prologues, preprocessor definitions, and comments that are unrelated to the definition @@ -49,9 +104,7 @@ def apply_ident( prompt = dedent( prompt ).strip() # note: dedent then format since the C definition isn't indented - prompt = prompt.format( - identifier=identifier, c_definition=c_definition.effective - ) + prompt = prompt.format(identifier=identifier, c_definition=definition) messages = [ {"role": "user", "content": prompt}, @@ -59,30 +112,41 @@ def apply_ident( transform = self.__class__.__name__ model = self.model.id - if response := self.cache.lookup( + response = self.cache.lookup( transform=transform, identifier=identifier, model=model, messages=messages, - ): - return response + ) + cache_hit = response is not None - response = self.model.generate_with_tools(messages) + if response is None: + response = self.model.generate_with_tools(messages) if response is None: logging.error("Model returned no response") - return response + return None - # TODO: validate that function definition is still present? + trimmed = remove_backticks(response) - response = remove_backticks(response) + # A correct trim is a trailing slice of its input. This also keeps a + # response with hallucinated comment edits from becoming the ground + # truth that comment transfer is later validated against. + if not _is_suffix_ignoring_whitespace(trimmed, definition): + logging.warning( + f"{self.__class__.__name__}: " + f"response is not a trailing slice of the input for {identifier};" + " ignoring it" + ) + return None - self.cache.update( - transform=transform, - identifier=identifier, - model=model, - messages=messages, - response=response, - ) + if not cache_hit: + self.cache.update( + transform=transform, + identifier=identifier, + model=model, + messages=messages, + response=response, + ) - return response + return trimmed diff --git a/c2rust-postprocess/tests/test_comments_transform.py b/c2rust-postprocess/tests/test_comments_transform.py index 2c9fabc41e..db7367dcc7 100644 --- a/c2rust-postprocess/tests/test_comments_transform.py +++ b/c2rust-postprocess/tests/test_comments_transform.py @@ -8,11 +8,12 @@ class StaticCache(AbstractCache): + """Cache with a single response for `CommentsTransform` lookups.""" + def __init__(self, response: str): super().__init__(Path()) self.response = response - self.lookups = 0 - self.messages: list[list[dict[str, Any]]] = [] + self.lookups: list[tuple[str, list[dict[str, Any]]]] = [] def lookup( self, @@ -22,9 +23,10 @@ def lookup( model: str, messages: list[dict[str, Any]], ) -> str | None: - self.lookups += 1 - self.messages.append(messages) - return self.response + self.lookups.append((transform, messages)) + if transform == "CommentsTransform": + return self.response + return None def update( self, @@ -77,6 +79,7 @@ def test_directive_line_comment_survives_preprocessed_check() -> None: update_rust=False, ) - assert cache.lookups == 1 - prompt = cache.messages[0][0]["content"] + transforms = [transform for transform, _ in cache.lookups] + assert transforms == ["TrimTransform", "CommentsTransform"] + prompt = cache.lookups[-1][1][0]["content"] assert "preprocessor directive lines" in prompt diff --git a/c2rust-postprocess/tests/test_trim_transform.py b/c2rust-postprocess/tests/test_trim_transform.py new file mode 100644 index 0000000000..a745ed12fb --- /dev/null +++ b/c2rust-postprocess/tests/test_trim_transform.py @@ -0,0 +1,141 @@ +from pathlib import Path +from typing import Any + +from postprocess.cache import AbstractCache +from postprocess.definitions import CDefinition +from postprocess.models.mock import MockGenerativeModel +from postprocess.transforms.trim import ( + TrimTransform, + _is_suffix_ignoring_whitespace, + _prefix_needs_trim, +) + +DOC_COMMENT_ONLY = """\ +/* +** doc comment for f +*/ +int f(void) { + return 0; +} +""" + +LICENSE_THEN_DOC = """\ +/* Copyright (c) 2026 Example Corp. */ + +/* doc comment for f */ +int f(void) { + return 0; +} +""" + + +def test_prefix_without_comments_needs_no_trim() -> None: + definition = "int f(void) {\n return 0;\n}\n" + assert not _prefix_needs_trim(definition, 0) + + definition = "#include \n\nint f(void) {\n return 0;\n}\n" + assert not _prefix_needs_trim(definition, 2) + + +def test_adjacent_doc_comment_needs_no_trim() -> None: + assert not _prefix_needs_trim(DOC_COMMENT_ONLY, 3) + + +def test_blank_separated_doc_comment_needs_no_trim() -> None: + definition = "/* doc comment for f */\n\nint f(void) {\n return 0;\n}\n" + assert not _prefix_needs_trim(definition, 2) + + +def test_directives_above_doc_comment_need_no_trim() -> None: + definition = "#include \n\n/* doc */\nint f(void) {}\n" + assert not _prefix_needs_trim(definition, 3) + + +def test_license_above_doc_comment_needs_trim() -> None: + assert _prefix_needs_trim(LICENSE_THEN_DOC, 3) + + +def test_commented_directive_needs_trim() -> None: + definition = "#define MAX 100 /* max entries */\nint f(void) {}\n" + assert _prefix_needs_trim(definition, 1) + + definition = "/* max entries */\n#define MAX 100\nint f(void) {}\n" + assert _prefix_needs_trim(definition, 2) + + +def test_is_suffix_ignoring_whitespace() -> None: + original = LICENSE_THEN_DOC + trimmed = "/* doc comment for f */\nint f(void) {\n return 0;\n}" + assert _is_suffix_ignoring_whitespace(trimmed, original) + assert _is_suffix_ignoring_whitespace(original, original) + # reformatted whitespace is fine, reworded text is not + assert _is_suffix_ignoring_whitespace("int f(void) { return 0; }", original) + assert not _is_suffix_ignoring_whitespace("/* doc for f */ int f(void)", original) + assert not _is_suffix_ignoring_whitespace("", original) + + +class RecordingCache(AbstractCache): + def __init__(self, response: str | None): + super().__init__(Path()) + self.response = response + self.lookups = 0 + self.updates = 0 + + def lookup( + self, + *, + transform: str, + identifier: str, + model: str, + messages: list[dict[str, Any]], + ) -> str | None: + self.lookups += 1 + return self.response + + def update( + self, + *, + transform: str, + identifier: str, + model: str, + messages: list[dict[str, Any]], + response: str, + ) -> None: + self.updates += 1 + + +def apply_trim(c_definition: CDefinition, cache: AbstractCache) -> str | None: + transform = TrimTransform(cache=cache, model=MockGenerativeModel()) + return transform.apply_ident( + rust_source_file=Path("unused.rs"), + rust_definition="fn f() {}", + c_definition=c_definition, + identifier="f", + update_rust=False, + ) + + +def test_gate_skips_model_when_no_trim_needed() -> None: + cache = RecordingCache("should not be used") + c_definition = CDefinition( + definition=DOC_COMMENT_ONLY, preprocessed_definition=None, decl_line=3 + ) + assert apply_trim(c_definition, cache) is None + assert cache.lookups == 0 + + +def test_trim_accepts_trailing_slice_response() -> None: + trimmed = "/* doc comment for f */\nint f(void) {\n return 0;\n}" + cache = RecordingCache(f"```c\n{trimmed}\n```") + c_definition = CDefinition( + definition=LICENSE_THEN_DOC, preprocessed_definition=None, decl_line=3 + ) + assert apply_trim(c_definition, cache) == trimmed + + +def test_trim_rejects_non_trailing_slice_response() -> None: + cache = RecordingCache("/* doc for f */\nint f(void) {\n return 0;\n}") + c_definition = CDefinition( + definition=LICENSE_THEN_DOC, preprocessed_definition=None, decl_line=3 + ) + assert apply_trim(c_definition, cache) is None From dfa762b64388b62abd499c9a330aa1710a5416b5 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Tue, 7 Jul 2026 20:53:54 -0700 Subject: [PATCH 06/13] postprocess: prompt with an explicit list of comments to transfer --- .../postprocess/definitions/__init__.py | 13 ------- .../postprocess/transforms/comments.py | 37 +++++++++---------- .../tests/test_comments_transform.py | 2 +- 3 files changed, 18 insertions(+), 34 deletions(-) diff --git a/c2rust-postprocess/postprocess/definitions/__init__.py b/c2rust-postprocess/postprocess/definitions/__init__.py index a1640e8096..6933facada 100644 --- a/c2rust-postprocess/postprocess/definitions/__init__.py +++ b/c2rust-postprocess/postprocess/definitions/__init__.py @@ -257,19 +257,6 @@ def effective(self) -> str: return self.preprocessed_definition return self.definition - @property - def was_changed_by_preprocessing(self) -> bool: - return ( - self.preprocessed_definition is not None - and self.preprocessed_definition != self.definition - ) - - @property - def preprocessed_if_changed(self) -> str | None: - if self.was_changed_by_preprocessing: - return self.preprocessed_definition - return None - def get_c_definitions(root_rust_source_file: Path) -> dict[str, CDefinition]: c_defs_json = root_rust_source_file.with_suffix(".c_decls.json") diff --git a/c2rust-postprocess/postprocess/transforms/comments.py b/c2rust-postprocess/postprocess/transforms/comments.py index 98ea7070b6..d4fd38a498 100644 --- a/c2rust-postprocess/postprocess/transforms/comments.py +++ b/c2rust-postprocess/postprocess/transforms/comments.py @@ -25,31 +25,25 @@ @dataclass(slots=True) class CommentsTransformPrompt: c_function: str - c_function_preprocessed: str | None + c_comments: list[str] rust_function: str prompt_text: str identifier: str def __str__(self) -> str: - prompt = ( + return ( self.prompt_text + "\n\n" + + "Comment lines to transfer:\n```\n" + + "\n".join(self.c_comments) + + "\n```\n\n" + "C function:\n```c\n" + self.c_function + "```\n\n" + + "Rust function:\n```rust\n" + + self.rust_function + + "```\n" ) - if self.c_function_preprocessed is not None: - prompt += ( - "The same C function after preprocessing; comments that are" - " absent here may have been in inactive preprocessor regions" - " and must not be transferred. Comments on preprocessor" - " directive lines in the original C function may be absent" - " here even when they annotate active code:\n```c\n" - + self.c_function_preprocessed - + "```\n\n" - ) - prompt += "Rust function:\n```rust\n" + self.rust_function + "```\n" - return prompt def _get_directive_line_comments(code: str) -> list[str]: @@ -114,11 +108,12 @@ def apply_ident( f"{identifier}; continuing with the original definition" ) case str() as trimmed_c_definition: - # TODO: consider trimming both the definition and the preprocessed - # definition instead of possibly replacing the original - # definition with the trimmed and preprocessed one. + # Keep the (untrimmed) preprocessed definition: it is only used + # as a comment multiset when computing transferable comments, + # so trimming it is unnecessary. c_definition = CDefinition( - definition=trimmed_c_definition, preprocessed_definition=None + definition=trimmed_c_definition, + preprocessed_definition=c_definition.preprocessed_definition, ) c_comments = _get_transferable_c_comments(c_definition) if not c_comments: @@ -132,7 +127,9 @@ def apply_ident( # TODO: make this function take a model and get prompt from model prompt_text = """ Transfer the comments from the following C function to the corresponding Rust function. - Do not add any comments that are not present in the C function. + Transfer exactly the comment lines listed below, in order; do not transfer any other + comments in the C function (they may come from inactive preprocessor regions) and do + not add new ones. Use Rust doc comment syntax (///) where appropriate (e.g., for function documentation). Respond with the Rust function definition with the transferred comments; say nothing else. """ # noqa: E501 @@ -140,7 +137,7 @@ def apply_ident( prompt = CommentsTransformPrompt( c_function=c_definition.definition, - c_function_preprocessed=c_definition.preprocessed_if_changed, + c_comments=c_comments, rust_function=rust_definition, prompt_text=prompt_text, identifier=identifier, diff --git a/c2rust-postprocess/tests/test_comments_transform.py b/c2rust-postprocess/tests/test_comments_transform.py index db7367dcc7..64479dc4b9 100644 --- a/c2rust-postprocess/tests/test_comments_transform.py +++ b/c2rust-postprocess/tests/test_comments_transform.py @@ -82,4 +82,4 @@ def test_directive_line_comment_survives_preprocessed_check() -> None: transforms = [transform for transform, _ in cache.lookups] assert transforms == ["TrimTransform", "CommentsTransform"] prompt = cache.lookups[-1][1][0]["content"] - assert "preprocessor directive lines" in prompt + assert "Comment lines to transfer:\n```\nenabled path\n```" in prompt From 77b53151a9d6a217f59c74853113b107c1062a35 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Wed, 8 Jul 2026 15:19:12 -0700 Subject: [PATCH 07/13] postprocess: reject syntactically invalid model responses --- .../postprocess/definitions/__init__.py | 6 +++ .../postprocess/transforms/comments.py | 6 +++ .../tests/test_comments_transform.py | 39 +++++++++++++++++++ c2rust-postprocess/tests/test_definitions.py | 6 +++ 4 files changed, 57 insertions(+) diff --git a/c2rust-postprocess/postprocess/definitions/__init__.py b/c2rust-postprocess/postprocess/definitions/__init__.py index 6933facada..1377b00995 100644 --- a/c2rust-postprocess/postprocess/definitions/__init__.py +++ b/c2rust-postprocess/postprocess/definitions/__init__.py @@ -206,6 +206,12 @@ def walk(node: Node) -> Generator[str]: return get_comments_text(walk(tree.root_node)) +def rust_parse_has_errors(code: str) -> bool: + """True if tree-sitter finds syntax errors in the given Rust code.""" + parser = Parser(RUST_LANGUAGE) + return parser.parse(code.encode()).root_node.has_error + + def is_comment_token(tok_type: Any) -> bool: """True for real comments; excludes directives and include paths.""" return tok_type in Comment and tok_type not in { diff --git a/c2rust-postprocess/postprocess/transforms/comments.py b/c2rust-postprocess/postprocess/transforms/comments.py index d4fd38a498..432949ad80 100644 --- a/c2rust-postprocess/postprocess/transforms/comments.py +++ b/c2rust-postprocess/postprocess/transforms/comments.py @@ -9,6 +9,7 @@ CDefinition, get_c_comments, get_rust_comments, + rust_parse_has_errors, update_rust_definition, ) from postprocess.models import AbstractGenerativeModel, api_key_from_env @@ -172,6 +173,11 @@ def apply_ident( rust_fn = remove_backticks(response) + if rust_parse_has_errors(rust_fn): + raise TransformError( + f"model response for {identifier} is not syntactically valid Rust" + ) + logging.debug(f"{c_comments=}") rust_comments = get_rust_comments(rust_fn) diff --git a/c2rust-postprocess/tests/test_comments_transform.py b/c2rust-postprocess/tests/test_comments_transform.py index 64479dc4b9..7094a8816b 100644 --- a/c2rust-postprocess/tests/test_comments_transform.py +++ b/c2rust-postprocess/tests/test_comments_transform.py @@ -1,9 +1,12 @@ from pathlib import Path from typing import Any +import pytest + from postprocess.cache import AbstractCache from postprocess.definitions import CDefinition from postprocess.models.mock import MockGenerativeModel +from postprocess.transforms.base import TransformError from postprocess.transforms.comments import CommentsTransform @@ -83,3 +86,39 @@ def test_directive_line_comment_survives_preprocessed_check() -> None: assert transforms == ["TrimTransform", "CommentsTransform"] prompt = cache.lookups[-1][1][0]["content"] assert "Comment lines to transfer:\n```\nenabled path\n```" in prompt + + +C_DEFINITION_BODY_COMMENT = CDefinition( + definition="""\ +int f(void) { + /// @note a body comment + return 1; +} +""", + preprocessed_definition=None, +) + +RUST_DEFINITION_NO_COMMENTS = """\ +pub unsafe extern "C" fn f() -> libc::c_int { + return 1 as libc::c_int; +} +""" + + +def test_syntactically_invalid_response_is_rejected() -> None: + response = """\ +pub unsafe extern "C" fn f( -> libc::c_int { + /// @note a body comment + return 1 as libc::c_int; +""" + cache = StaticCache(response) + transform = CommentsTransform(cache=cache, model=MockGenerativeModel()) + + with pytest.raises(TransformError, match="not syntactically valid"): + transform.apply_ident( + rust_source_file=Path("unused.rs"), + rust_definition=RUST_DEFINITION_NO_COMMENTS, + c_definition=C_DEFINITION_BODY_COMMENT, + identifier="f", + update_rust=False, + ) diff --git a/c2rust-postprocess/tests/test_definitions.py b/c2rust-postprocess/tests/test_definitions.py index ef754288a3..2d82ed3cf1 100644 --- a/c2rust-postprocess/tests/test_definitions.py +++ b/c2rust-postprocess/tests/test_definitions.py @@ -9,6 +9,7 @@ get_c_sourcefile, get_function_span_pairs, get_rust_function_spans, + rust_parse_has_errors, ) from postprocess.utils import read_chunk @@ -84,6 +85,11 @@ def test_c_function_splitting(generate_compile_commands_for_qsort, transpile_qso # assert False +def test_rust_parse_has_errors(): + assert not rust_parse_has_errors("fn f() -> i32 { 1 }\n") + assert rust_parse_has_errors("fn f( -> i32 { 1 }\n") + + def test_comment_insertion_qsort(): qsort_rs = Path(__file__).parent / "examples/qsort.rs" qsort_rs = qsort_rs.relative_to(Path.cwd()) From aad92c9378f2a5f38d7ff8a2835ae76e41b0647a Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Wed, 8 Jul 2026 15:19:49 -0700 Subject: [PATCH 08/13] postprocess: demote doc comments that don't document the function --- .../postprocess/definitions/__init__.py | 63 +++++++++++++++++++ .../postprocess/transforms/comments.py | 3 + .../tests/test_comments_transform.py | 30 +++++++++ c2rust-postprocess/tests/test_definitions.py | 40 ++++++++++++ 4 files changed, 136 insertions(+) diff --git a/c2rust-postprocess/postprocess/definitions/__init__.py b/c2rust-postprocess/postprocess/definitions/__init__.py index 1377b00995..e39bd22acc 100644 --- a/c2rust-postprocess/postprocess/definitions/__init__.py +++ b/c2rust-postprocess/postprocess/definitions/__init__.py @@ -206,6 +206,69 @@ def walk(node: Node) -> Generator[str]: return get_comments_text(walk(tree.root_node)) +def _plain_comment_prefix(comment: str) -> str | None: + """ + The plain-comment marker that replaces a doc-comment marker, + or None if `comment` is not a doc comment. + """ + # Per rustc: ////... and /***...*/ and the empty /**/ are plain comments. + if comment.startswith("//!"): + return "//" + if comment.startswith("///") and not comment.startswith("////"): + return "//" + if comment.startswith("/*!"): + return "/*" + if ( + comment.startswith("/**") + and not comment.startswith("/***") + and comment != "/**/" + ): + return "/*" + return None + + +def demote_misplaced_doc_comments(code: str) -> str: + """ + Rewrite doc comments as plain comments wherever rustc would reject or + ignore them in a function definition fragment. A /// before an expression + statement desugars to an attribute on the expression, which is a hard + error on stable (E0658, stmt_expr_attributes); elsewhere in a body it is + an unused_doc_comments warning. Inner doc comments (//!) are demoted + everywhere since the fragment is merged into the middle of a file. + """ + parser = Parser(RUST_LANGUAGE) + code_bytes = code.encode() + tree = parser.parse(code_bytes) + + replacements = [] + + def walk(node: Node) -> None: + if node.type in {"line_comment", "block_comment"}: + text = code_bytes[node.start_byte : node.end_byte].decode() + prefix = _plain_comment_prefix(text) + inner_doc = text.startswith(("//!", "/*!")) + misplaced = node.parent is None or node.parent.type != "source_file" + if prefix is not None and (inner_doc or misplaced): + # all doc-comment markers are 3 bytes + replacements.append((node.start_byte, node.end_byte, prefix + text[3:])) + for child in node.children: + walk(child) + + walk(tree.root_node) + + if not replacements: + return code + + out = [] + pos = 0 + for start, end, new_text in replacements: + out.append(code_bytes[pos:start].decode()) + out.append(new_text) + pos = end + out.append(code_bytes[pos:].decode()) + return "".join(out) + + def rust_parse_has_errors(code: str) -> bool: """True if tree-sitter finds syntax errors in the given Rust code.""" parser = Parser(RUST_LANGUAGE) diff --git a/c2rust-postprocess/postprocess/transforms/comments.py b/c2rust-postprocess/postprocess/transforms/comments.py index 432949ad80..df74ca4fb3 100644 --- a/c2rust-postprocess/postprocess/transforms/comments.py +++ b/c2rust-postprocess/postprocess/transforms/comments.py @@ -7,6 +7,7 @@ from postprocess.cache import AbstractCache from postprocess.definitions import ( CDefinition, + demote_misplaced_doc_comments, get_c_comments, get_rust_comments, rust_parse_has_errors, @@ -178,6 +179,8 @@ def apply_ident( f"model response for {identifier} is not syntactically valid Rust" ) + rust_fn = demote_misplaced_doc_comments(rust_fn) + logging.debug(f"{c_comments=}") rust_comments = get_rust_comments(rust_fn) diff --git a/c2rust-postprocess/tests/test_comments_transform.py b/c2rust-postprocess/tests/test_comments_transform.py index 7094a8816b..1ba8730c2e 100644 --- a/c2rust-postprocess/tests/test_comments_transform.py +++ b/c2rust-postprocess/tests/test_comments_transform.py @@ -6,6 +6,7 @@ from postprocess.cache import AbstractCache from postprocess.definitions import CDefinition from postprocess.models.mock import MockGenerativeModel +from postprocess.transforms import comments from postprocess.transforms.base import TransformError from postprocess.transforms.comments import CommentsTransform @@ -105,6 +106,35 @@ def test_directive_line_comment_survives_preprocessed_check() -> None: """ +def test_body_doc_comments_are_demoted(monkeypatch) -> None: + response = """\ +pub unsafe extern "C" fn f() -> libc::c_int { + /// @note a body comment + return 1 as libc::c_int; +} +""" + cache = StaticCache(response) + transform = CommentsTransform(cache=cache, model=MockGenerativeModel()) + + merged: dict[str, str] = {} + + def fake_update(*, root_rust_source_file, identifier, new_definition): + merged[identifier] = new_definition + + monkeypatch.setattr(comments, "update_rust_definition", fake_update) + + transform.apply_ident( + rust_source_file=Path("unused.rs"), + rust_definition=RUST_DEFINITION_NO_COMMENTS, + c_definition=C_DEFINITION_BODY_COMMENT, + identifier="f", + update_rust=True, + ) + + assert "/// @note" not in merged["f"] + assert "// @note a body comment" in merged["f"] + + def test_syntactically_invalid_response_is_rejected() -> None: response = """\ pub unsafe extern "C" fn f( -> libc::c_int { diff --git a/c2rust-postprocess/tests/test_definitions.py b/c2rust-postprocess/tests/test_definitions.py index 2d82ed3cf1..953bfa8738 100644 --- a/c2rust-postprocess/tests/test_definitions.py +++ b/c2rust-postprocess/tests/test_definitions.py @@ -6,6 +6,7 @@ from postprocess import main from postprocess.definitions import ( + demote_misplaced_doc_comments, get_c_sourcefile, get_function_span_pairs, get_rust_function_spans, @@ -85,6 +86,45 @@ def test_c_function_splitting(generate_compile_commands_for_qsort, transpile_qso # assert False +def test_demote_misplaced_doc_comments(): + code = """\ +#[no_mangle] +/// doc on fn stays +pub unsafe extern "C" fn f() -> i32 { + /// doc before let + let mut x = 0; + /// doc before expr stmt + x = 1; + /** block doc */ + x = 2; + //// four slashes is a plain comment + // plain comment + return x; +} +""" + expected = """\ +#[no_mangle] +/// doc on fn stays +pub unsafe extern "C" fn f() -> i32 { + // doc before let + let mut x = 0; + // doc before expr stmt + x = 1; + /* block doc */ + x = 2; + //// four slashes is a plain comment + // plain comment + return x; +} +""" + assert demote_misplaced_doc_comments(code) == expected + + +def test_demote_misplaced_doc_comments_inner_doc(): + code = "//! inner doc\nfn f() {}\n" + assert demote_misplaced_doc_comments(code) == "// inner doc\nfn f() {}\n" + + def test_rust_parse_has_errors(): assert not rust_parse_has_errors("fn f() -> i32 { 1 }\n") assert rust_parse_has_errors("fn f( -> i32 { 1 }\n") From 579ba1f15668b6b166cbdf46aadf9bb0c2dbc129 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Wed, 8 Jul 2026 15:20:03 -0700 Subject: [PATCH 09/13] postprocess: tell the model to keep /// out of function bodies --- c2rust-postprocess/postprocess/transforms/comments.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/c2rust-postprocess/postprocess/transforms/comments.py b/c2rust-postprocess/postprocess/transforms/comments.py index df74ca4fb3..a3a8bdfbb8 100644 --- a/c2rust-postprocess/postprocess/transforms/comments.py +++ b/c2rust-postprocess/postprocess/transforms/comments.py @@ -132,7 +132,8 @@ def apply_ident( Transfer exactly the comment lines listed below, in order; do not transfer any other comments in the C function (they may come from inactive preprocessor regions) and do not add new ones. - Use Rust doc comment syntax (///) where appropriate (e.g., for function documentation). + Use Rust doc comment syntax (///) only for comments placed before the function signature; + inside the function body use plain // comments (/// before a statement does not compile). Respond with the Rust function definition with the transferred comments; say nothing else. """ # noqa: E501 prompt_text = dedent(prompt_text).strip() From 8d23ea989dd55c05a5dd283a297642a9dda462e6 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Thu, 9 Jul 2026 04:17:08 -0700 Subject: [PATCH 10/13] postprocess: regenerate rejected responses; cache only validated ones --- .../postprocess/transforms/base.py | 131 +++++++++++++++++- .../postprocess/transforms/comments.py | 89 ++++-------- .../postprocess/transforms/trim.py | 59 +++----- .../tests/test_comments_transform.py | 107 +++++++++++++- 4 files changed, 272 insertions(+), 114 deletions(-) diff --git a/c2rust-postprocess/postprocess/transforms/base.py b/c2rust-postprocess/postprocess/transforms/base.py index 6a3c112be8..580895b008 100644 --- a/c2rust-postprocess/postprocess/transforms/base.py +++ b/c2rust-postprocess/postprocess/transforms/base.py @@ -1,14 +1,18 @@ import logging import re -from abc import ABC, abstractmethod +from collections.abc import Callable from pathlib import Path +from typing import Any +from postprocess.cache import AbstractCache from postprocess.definitions import ( CDefinition, get_c_definitions, get_rust_definitions, + update_rust_definition, ) from postprocess.exclude_list import IdentifierExcludeList +from postprocess.models import AbstractGenerativeModel, api_key_from_env from postprocess.utils import get_highlighted_c @@ -16,19 +20,27 @@ class TransformError(Exception): """A transform failed to process a single definition.""" -class AbstractTransform(ABC): +class AbstractTransform: """ Abstract base class for LLM-driven transforms of c2rust transpiler output. """ - def __init__(self, system_instruction: str): + max_attempts = 3 + + def __init__( + self, + system_instruction: str, + cache: AbstractCache, + model: AbstractGenerativeModel, + ): self._system_instruction = system_instruction + self.cache = cache + self.model = model @property def system_instruction(self) -> str: return self._system_instruction - @abstractmethod def apply_ident( self, rust_source_file: Path, @@ -38,10 +50,115 @@ def apply_ident( update_rust: bool = True, ) -> str | None: """ - Implementations should apply transform to a single Rust definition - with the given identifier. + Apply the transform to one Rust definition and commit it through + merge_rust. + """ + new_definition = self.try_apply_ident( + rust_source_file=rust_source_file, + rust_definition=rust_definition, + c_definition=c_definition, + identifier=identifier, + ) + if new_definition is None: + return None + + if new_definition == rust_definition: + logging.info( + f"{self.__class__.__name__}: No changes for function: {identifier}" + ) + return None + + if update_rust: + update_rust_definition( + root_rust_source_file=rust_source_file, + identifier=identifier, + new_definition=new_definition, + ) + + logging.info(f"{self.__class__.__name__}: Updated Rust fn {identifier}") + return new_definition + + def try_apply_ident( + self, + rust_source_file: Path, + rust_definition: str, + c_definition: CDefinition, + identifier: str, + ) -> str | None: + """ + Return a replacement Rust definition for `identifier`, + or None to skip it. + """ + raise NotImplementedError( + f"{self.__class__.__name__} must implement try_apply_ident, " + "or override apply_ident directly" + ) + + def generate( + self, + identifier: str, + messages: list[dict[str, Any]], + check: Callable[[str], str], + ) -> str | None: + """ + Model call with caching, validation, and retries. `check` returns the + validated result or raises TransformError to trigger regeneration. """ - pass + transform = self.__class__.__name__ + error: TransformError | None = None + + response = self.cache.lookup( + transform=transform, + identifier=identifier, + model=self.model.id, + messages=messages, + ) + if response is not None: + try: + return check(response) + except TransformError as stale: + error = stale + logging.warning( + f"{transform}: cached response for {identifier} " + f"failed validation: {stale}" + ) + + if api_key_from_env(self.model.id) is None: + if error is not None: + # Can't regenerate the invalid cached response without a key. + raise error + logging.warning( + f"Cache miss for {identifier}; skipping since no API key was set..." + ) + return None + + for attempt in range(self.max_attempts): + try: + response = self.model.generate_with_tools(messages) + if response is None: + raise TransformError(f"model returned no response for {identifier}") + result = check(response) + except TransformError as rejected: + error = rejected + logging.warning( + f"{transform}: response for {identifier} rejected on attempt " + f"{attempt + 1}/{self.max_attempts}: {rejected}" + ) + continue + + self.cache.update( + transform=transform, + identifier=identifier, + model=self.model.id, + messages=messages, + response=response, + ) + return result + + raise TransformError( + f"{transform} failed after {self.max_attempts} attempts " + f"for {identifier}: {error}" + ) from error def apply_dir( self, diff --git a/c2rust-postprocess/postprocess/transforms/comments.py b/c2rust-postprocess/postprocess/transforms/comments.py index a3a8bdfbb8..27687cd5a2 100644 --- a/c2rust-postprocess/postprocess/transforms/comments.py +++ b/c2rust-postprocess/postprocess/transforms/comments.py @@ -11,9 +11,8 @@ get_c_comments, get_rust_comments, rust_parse_has_errors, - update_rust_definition, ) -from postprocess.models import AbstractGenerativeModel, api_key_from_env +from postprocess.models import AbstractGenerativeModel from postprocess.transforms.base import AbstractTransform, TransformError from postprocess.transforms.trim import TrimTransform from postprocess.utils import get_highlighted_rust, remove_backticks @@ -71,31 +70,28 @@ def _get_transferable_c_comments(c_definition: CDefinition) -> list[str]: class CommentsTransform(AbstractTransform): def __init__(self, cache: AbstractCache, model: AbstractGenerativeModel): - super().__init__(SYSTEM_INSTRUCTION) - self.cache = cache - self.model = model + super().__init__(SYSTEM_INSTRUCTION, cache, model) self.trim_transform = TrimTransform(cache, model) - def apply_ident( + def try_apply_ident( self, rust_source_file: Path, rust_definition: str, c_definition: CDefinition, identifier: str, - update_rust: bool = True, - ) -> None: + ) -> str | None: rust_comments = get_rust_comments(rust_definition) if rust_comments: logging.info( f"Skipping Rust fn {identifier} with existing comments:\ \n{get_highlighted_rust(rust_definition)}" ) - return + return None c_comments = _get_transferable_c_comments(c_definition) if not c_comments: logging.info(f"Skipping C function without comments: {identifier}") - return + return None match self.trim_transform.apply_ident( rust_source_file=rust_source_file, @@ -120,7 +116,7 @@ def apply_ident( c_comments = _get_transferable_c_comments(c_definition) if not c_comments: logging.info(f"Skipping C function without comments: {identifier}") - return + return None case _: raise AssertionError( "Unexpected return type from trim transform: expected None or str" @@ -150,68 +146,33 @@ def apply_ident( {"role": "user", "content": str(prompt)}, ] - transform = self.__class__.__name__ - identifier = prompt.identifier - model = self.model.id - response = self.cache.lookup( - transform=transform, - identifier=identifier, - model=model, - messages=messages, - ) - cache_hit = response is not None - - if response is None: - response = self.model.generate_with_tools(messages) + def check(response: str) -> str: + rust_fn = remove_backticks(response) - if response is None: - if api_key_from_env(model) is None: - # No API key set: skip uncached entries instead of failing. - logging.warning( - f"Cache miss for {identifier}; skipping since no API key was set..." + if rust_parse_has_errors(rust_fn): + raise TransformError( + f"model response for {identifier} is not syntactically valid " + f"Rust:\n```rust\n{rust_fn}\n```" ) - return - raise TransformError(f"model returned no response for {identifier}") - - rust_fn = remove_backticks(response) - - if rust_parse_has_errors(rust_fn): - raise TransformError( - f"model response for {identifier} is not syntactically valid Rust" - ) - rust_fn = demote_misplaced_doc_comments(rust_fn) + rust_fn = demote_misplaced_doc_comments(rust_fn) - logging.debug(f"{c_comments=}") + rust_comments = get_rust_comments(rust_fn) + if c_comments != rust_comments: + raise TransformError( + f"comments were not transferred verbatim for {identifier}:" + f"\n{c_comments=}\n{rust_comments=}" + ) - rust_comments = get_rust_comments(rust_fn) - logging.debug(f"{rust_comments=}") + return rust_fn - if c_comments != rust_comments: - raise TransformError( - f"comments were not transferred verbatim for {identifier}:" - f"\n{c_comments=}\n{rust_comments=}" - ) - - if not cache_hit: - self.cache.update( - transform=transform, - identifier=identifier, - model=model, - messages=messages, - response=response, - ) + rust_fn = self.generate(identifier, messages, check) + if rust_fn is None: + return None logging.info( f"Comments transferred to Rust fn {identifier}:\ \n{get_highlighted_rust(rust_fn)}" ) - # TODO: move this to apply_file? - # the challenge is that not all transforms will update Rust code - if update_rust: - update_rust_definition( - root_rust_source_file=rust_source_file, - identifier=prompt.identifier, - new_definition=rust_fn, - ) + return rust_fn diff --git a/c2rust-postprocess/postprocess/transforms/trim.py b/c2rust-postprocess/postprocess/transforms/trim.py index 5c9467641e..3a73fb5577 100644 --- a/c2rust-postprocess/postprocess/transforms/trim.py +++ b/c2rust-postprocess/postprocess/transforms/trim.py @@ -8,7 +8,7 @@ from postprocess.cache import AbstractCache from postprocess.definitions import CDefinition, get_c_comments, is_comment_token from postprocess.models import AbstractGenerativeModel -from postprocess.transforms.base import AbstractTransform +from postprocess.transforms.base import AbstractTransform, TransformError from postprocess.utils import remove_backticks SYSTEM_INSTRUCTION = ( @@ -59,9 +59,7 @@ def _is_suffix_ignoring_whitespace(trimmed: str, original: str) -> bool: class TrimTransform(AbstractTransform): def __init__(self, cache: AbstractCache, model: AbstractGenerativeModel): - super().__init__(SYSTEM_INSTRUCTION) - self.cache = cache - self.model = model + super().__init__(SYSTEM_INSTRUCTION, cache, model) def apply_ident( self, @@ -110,43 +108,22 @@ def apply_ident( {"role": "user", "content": prompt}, ] - transform = self.__class__.__name__ - model = self.model.id - response = self.cache.lookup( - transform=transform, - identifier=identifier, - model=model, - messages=messages, - ) - cache_hit = response is not None - - if response is None: - response = self.model.generate_with_tools(messages) - - if response is None: - logging.error("Model returned no response") - return None + def check(response: str) -> str: + trimmed = remove_backticks(response) - trimmed = remove_backticks(response) + # A correct trim is a trailing slice of its input. This also keeps a + # response with hallucinated comment edits from becoming the ground + # truth that comment transfer is later validated against. + if not _is_suffix_ignoring_whitespace(trimmed, definition): + raise TransformError( + f"response is not a trailing slice of the input for {identifier}" + ) - # A correct trim is a trailing slice of its input. This also keeps a - # response with hallucinated comment edits from becoming the ground - # truth that comment transfer is later validated against. - if not _is_suffix_ignoring_whitespace(trimmed, definition): - logging.warning( - f"{self.__class__.__name__}: " - f"response is not a trailing slice of the input for {identifier};" - " ignoring it" - ) - return None + return trimmed - if not cache_hit: - self.cache.update( - transform=transform, - identifier=identifier, - model=model, - messages=messages, - response=response, - ) - - return trimmed + try: + return self.generate(identifier, messages, check) + except TransformError as error: + # Trimming is best-effort; callers fall back to the untrimmed input. + logging.warning(f"{self.__class__.__name__}: {error}") + return None diff --git a/c2rust-postprocess/tests/test_comments_transform.py b/c2rust-postprocess/tests/test_comments_transform.py index 1ba8730c2e..9807ce4dcf 100644 --- a/c2rust-postprocess/tests/test_comments_transform.py +++ b/c2rust-postprocess/tests/test_comments_transform.py @@ -6,7 +6,7 @@ from postprocess.cache import AbstractCache from postprocess.definitions import CDefinition from postprocess.models.mock import MockGenerativeModel -from postprocess.transforms import comments +from postprocess.transforms import base from postprocess.transforms.base import TransformError from postprocess.transforms.comments import CommentsTransform @@ -97,6 +97,7 @@ def test_directive_line_comment_survives_preprocessed_check() -> None: } """, preprocessed_definition=None, + decl_line=0, # no front matter, so the trim transform skips the model ) RUST_DEFINITION_NO_COMMENTS = """\ @@ -121,7 +122,7 @@ def test_body_doc_comments_are_demoted(monkeypatch) -> None: def fake_update(*, root_rust_source_file, identifier, new_definition): merged[identifier] = new_definition - monkeypatch.setattr(comments, "update_rust_definition", fake_update) + monkeypatch.setattr(base, "update_rust_definition", fake_update) transform.apply_ident( rust_source_file=Path("unused.rs"), @@ -152,3 +153,105 @@ def test_syntactically_invalid_response_is_rejected() -> None: identifier="f", update_rust=False, ) + + +class RecordingCache(AbstractCache): + """Cache with a fixed `CommentsTransform` response that records updates.""" + + def __init__(self, response: str | None): + super().__init__(Path()) + self.response = response + self.updates: list[tuple[list[dict[str, Any]], str]] = [] + + def lookup( + self, + *, + transform: str, + identifier: str, + model: str, + messages: list[dict[str, Any]], + ) -> str | None: + if transform != "CommentsTransform": + return None + return self.response + + def update( + self, + *, + transform: str, + identifier: str, + model: str, + messages: list[dict[str, Any]], + response: str, + ) -> None: + self.updates.append((messages, response)) + + +class QueuedModel(MockGenerativeModel): + """Mock model that returns queued responses.""" + + def __init__(self, responses: list[str]): + super().__init__() + self.responses = responses + self.calls = 0 + + def generate_with_tools(self, messages, tools=(), max_tool_loops=5): + self.calls += 1 + return self.responses.pop(0) + + +BAD_RESPONSE = """\ +pub unsafe extern "C" fn f( -> libc::c_int { + /// @note a body comment + return 1 as libc::c_int; +""" +GOOD_RESPONSE = """\ +pub unsafe extern "C" fn f() -> libc::c_int { + /// @note a body comment + return 1 as libc::c_int; +} +""" + + +def apply_to_body_comment_fn( + cache: AbstractCache, model: MockGenerativeModel +) -> str | None: + transform = CommentsTransform(cache=cache, model=model) + return transform.apply_ident( + rust_source_file=Path("unused.rs"), + rust_definition=RUST_DEFINITION_NO_COMMENTS, + c_definition=C_DEFINITION_BODY_COMMENT, + identifier="f", + update_rust=False, + ) + + +def test_rejected_response_is_regenerated(monkeypatch) -> None: + monkeypatch.setattr(base, "api_key_from_env", lambda model_id: "test-key") + cache = RecordingCache(None) + model = QueuedModel([BAD_RESPONSE, GOOD_RESPONSE]) + + result = apply_to_body_comment_fn(cache, model) + + assert result is not None + assert "// @note a body comment" in result + assert model.calls == 2 + # Only the validated response is cached, keyed by the unchanged prompt. + [(messages, response)] = cache.updates + assert len(messages) == 1 + assert response == GOOD_RESPONSE + + +def test_invalid_cached_response_is_regenerated(monkeypatch) -> None: + monkeypatch.setattr(base, "api_key_from_env", lambda model_id: "test-key") + cache = RecordingCache(BAD_RESPONSE) + model = QueuedModel([GOOD_RESPONSE]) + + result = apply_to_body_comment_fn(cache, model) + + assert result is not None + assert model.calls == 1 + # The stale entry is overwritten under the same key. + [(messages, response)] = cache.updates + assert len(messages) == 1 + assert response == GOOD_RESPONSE From 1f5168a382a212575723a74bd723d3118bbe5b1c Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Thu, 9 Jul 2026 16:13:24 -0700 Subject: [PATCH 11/13] postprocess: use tree-sitter-rust cast-comparison fix --- c2rust-postprocess/pyproject.toml | 4 ++++ c2rust-postprocess/uv.lock | 15 ++------------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/c2rust-postprocess/pyproject.toml b/c2rust-postprocess/pyproject.toml index 38621fce61..b8fb6682f0 100644 --- a/c2rust-postprocess/pyproject.toml +++ b/c2rust-postprocess/pyproject.toml @@ -22,6 +22,10 @@ postprocess = "postprocess.__main__:main" # CLI entry point # let `uv` to manage the environment package = true +[tool.uv.sources] +# TODO: Remove this override once tree-sitter-rust PR #312 merges upstream. +tree-sitter-rust = { git = "https://github.com/thedataking/tree-sitter-rust.git", rev = "1d3d5d6df51d5edfac3bdd3178433eb0aa437ea5" } + [dependency-groups] dev = [ "pytest>=9.0.3", diff --git a/c2rust-postprocess/uv.lock b/c2rust-postprocess/uv.lock index 1edc61057c..3b38369890 100644 --- a/c2rust-postprocess/uv.lock +++ b/c2rust-postprocess/uv.lock @@ -479,7 +479,7 @@ requires-dist = [ { name = "tomli", specifier = ">=2.3.0" }, { name = "tomlkit", specifier = ">=0.13.3" }, { name = "tree-sitter", specifier = ">=0.25.2" }, - { name = "tree-sitter-rust", specifier = ">=0.24.0" }, + { name = "tree-sitter-rust", git = "https://github.com/thedataking/tree-sitter-rust.git?rev=1d3d5d6df51d5edfac3bdd3178433eb0aa437ea5" }, ] [package.metadata.requires-dev] @@ -836,18 +836,7 @@ wheels = [ [[package]] name = "tree-sitter-rust" version = "0.24.2" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/b7/87/75cbd22b927267d310f76cca1ab3c1d9d41035dfa3eb9cc95f96ee199440/tree_sitter_rust-0.24.2.tar.gz", hash = "sha256:54fb02a5911e345308b405174465112479f56dc39e3f1e7744d7568595f00db9", size = 339341, upload-time = "2026-03-27T21:08:55.629Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/d0/24/2b2d33af5e27c84a4fde4e8cd2594bb4ab1e1cf48756a9f40dadc84956cc/tree_sitter_rust-0.24.2-cp39-abi3-macosx_10_9_x86_64.whl", hash = "sha256:3620cfd12340efa43082d45df76349ff511893a9c361da2f8d6d51e307020a59", size = 129507, upload-time = "2026-03-27T21:08:47.585Z" }, - { url = "https://files.pythonhosted.org/packages/78/2a/cf39f881a545360b5a86bb1accba1f4acc713daab01fb9edd35b6e84f473/tree_sitter_rust-0.24.2-cp39-abi3-macosx_11_0_arm64.whl", hash = "sha256:01a46622735498493f29f3e628a90de95c96a07bfbeb88996243eb986b1cee36", size = 136812, upload-time = "2026-03-27T21:08:48.761Z" }, - { url = "https://files.pythonhosted.org/packages/ca/45/a051bbd3045a61182dde25b93ae9a33d2677c935b16952283e12eaf46051/tree_sitter_rust-0.24.2-cp39-abi3-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:e033c5a93b57c88e0a835880de39fc802909ff69f57aaff6000211c196ea5190", size = 164706, upload-time = "2026-03-27T21:08:49.605Z" }, - { url = "https://files.pythonhosted.org/packages/b5/f6/a5a146df5c0a5daea3ffcd5d7245775fe7f084357770d5a313dd6245ae78/tree_sitter_rust-0.24.2-cp39-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:9d76d1208c3638b871236090759dfc13d478921320653a6c9da5336e7c58f65a", size = 170310, upload-time = "2026-03-27T21:08:50.424Z" }, - { url = "https://files.pythonhosted.org/packages/95/a8/f85b1ca75e01361ca5f92d226593ca4857cea49551b9f6c8fa6fc08ea917/tree_sitter_rust-0.24.2-cp39-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:87930163a462408c49ab62c667e74029bc26b4cc7123dd1bdc7352215786c64a", size = 168668, upload-time = "2026-03-27T21:08:51.404Z" }, - { url = "https://files.pythonhosted.org/packages/a2/e1/3519f866a4679ca36acd9f5a06a779ecb8a92b18887c5546458d521df557/tree_sitter_rust-0.24.2-cp39-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:da2b86099028fd42c6cd32878b7b16b01f8aac0f7b0e98742b7fa6bc3cf09b89", size = 162403, upload-time = "2026-03-27T21:08:52.588Z" }, - { url = "https://files.pythonhosted.org/packages/34/71/7ef609894dbfe5699eb16f7471f9b8af1d958d8ba3e29c238d7607e8cb47/tree_sitter_rust-0.24.2-cp39-abi3-win_amd64.whl", hash = "sha256:4529c125d928882ddfb879fdc6bc0704913261ecc078b6fa7902559e0daf200d", size = 129422, upload-time = "2026-03-27T21:08:54.031Z" }, - { url = "https://files.pythonhosted.org/packages/b9/d8/050a781172745bc345f98abb7c56e72022ea0790f8e793de981c83c2ef15/tree_sitter_rust-0.24.2-cp39-abi3-win_arm64.whl", hash = "sha256:66ba90f61bd54f4c4f5d30434957daf64507c16b0313df76becb37d63f70a227", size = 128245, upload-time = "2026-03-27T21:08:54.803Z" }, -] +source = { git = "https://github.com/thedataking/tree-sitter-rust.git?rev=1d3d5d6df51d5edfac3bdd3178433eb0aa437ea5#1d3d5d6df51d5edfac3bdd3178433eb0aa437ea5" } [[package]] name = "ty" From 85cb29c60d8b685bc062c38e7388433ae1af2edd Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Thu, 11 Jun 2026 00:31:32 -0700 Subject: [PATCH 12/13] tests: enable postprocessing for lua integration test --- tests/integration/tests/lua/conf.yml | 6 ++++++ .../integration/tests/lua/postprocess-exclude.yml | 14 ++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/integration/tests/lua/postprocess-exclude.yml diff --git a/tests/integration/tests/lua/conf.yml b/tests/integration/tests/lua/conf.yml index 5da76c055c..82220e45bf 100644 --- a/tests/integration/tests/lua/conf.yml +++ b/tests/integration/tests/lua/conf.yml @@ -33,3 +33,9 @@ refactor: cargo.refactor: autogen: true + +postprocess: + autogen: true + +cargo.postprocess: + autogen: true diff --git a/tests/integration/tests/lua/postprocess-exclude.yml b/tests/integration/tests/lua/postprocess-exclude.yml new file mode 100644 index 0000000000..b2917e5cb9 --- /dev/null +++ b/tests/integration/tests/lua/postprocess-exclude.yml @@ -0,0 +1,14 @@ +repo/src/lobject.rs: + - luaO_pushvfstring # repeated comment-order failures + +repo/src/lua.rs: + - collectargs # repeated comment-order failures + +repo/src/lstrlib.rs: + - match_0 # repeated comment-order failures + +repo/src/ldo.rs: + - luaD_call # repeated comment-order failures + +repo/src/lvm.rs: + - luaV_execute # main interpreter loop; invalid Rust output From 533487c8544040e3276744cfa71681fb9e12c394 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Thu, 9 Jul 2026 16:42:07 -0700 Subject: [PATCH 13/13] tests: narrow json-c postprocess exclusions --- .../tests/json-c/postprocess-exclude.yml | 25 +------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/tests/integration/tests/json-c/postprocess-exclude.yml b/tests/integration/tests/json-c/postprocess-exclude.yml index 9829c7da4f..b7553ec387 100644 --- a/tests/integration/tests/json-c/postprocess-exclude.yml +++ b/tests/integration/tests/json-c/postprocess-exclude.yml @@ -1,25 +1,2 @@ -repo/src/printbuf.rs: - - printbuf_extend - -repo/src/json_c_version.rs: - - json_c_version - -repo/src/json_pointer.rs: - - string_replace_all_occurrences_with_char - -repo/src/arraylist.rs: - - array_list_new - -repo/src/random_seed.rs: - - get_time_seed - repo/src/json_tokener.rs: - - json_tokener_parse_ex - -repo/src/linkhash.rs: - - hashlittle - -repo/src/json_object.rs: - - json_object_new - - get_string_component - - json_object_generic_delete + - json_tokener_parse_ex # large parser; invalid Rust output