Skip to content

postprocess: Add TrimTransform and enable postprocessing for lua#1849

Open
thedataking wants to merge 3 commits into
perl/postprocess-json-c-gpt-5.5-2026-04-23from
perl/postprocess-lua
Open

postprocess: Add TrimTransform and enable postprocessing for lua#1849
thedataking wants to merge 3 commits into
perl/postprocess-json-c-gpt-5.5-2026-04-23from
perl/postprocess-lua

Conversation

@thedataking

Copy link
Copy Markdown
Contributor

No description provided.

@thedataking thedataking changed the title Perl/postprocess lua postprocess: Add TrimTransform and enable postprocessing for lua Jun 11, 2026
@thedataking thedataking marked this pull request as draft June 11, 2026 07:39
@thedataking thedataking force-pushed the perl/postprocess-lua branch from c012824 to b35967c Compare June 11, 2026 07:55
@thedataking

thedataking commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Interesting failure I observed:

INFO:root:Comments transferred to Rust fn reverse:                
/// Reverse the stack segment from 'from' to 'to'
/// (auxiliary to 'lua_rotate')
/// Note that we move(copy) only the value inside the stack.
/// (We do not move additional fields that may exist.)
unsafe extern "C" fn reverse(mut L: *mut lua_State, mut from: StkId, mut to: StkId) {
    while from < to {
        let mut temp: TValue = TValue { value_:   Value { gc:   ::core::ptr::null_mut::<GCObject>() }, tt_:   0 };
        let mut io1: *mut TValue = &raw mut temp;
        let mut io2: *const TValue = &raw mut (*from).val;
        (*io1).value_ = (*io2).value_;
        (*io1).tt_ = (*io2).tt_;
        let mut io1_0: *mut TValue = &raw mut (*from).val;
        let mut io2_0: *const TValue = &raw mut (*to).val;
        (*io1_0).value_ = (*io2_0).value_;
        (*io1_0).tt_ = (*io2_0).tt_;
        let mut io1_1: *mut TValue = &raw mut (*to).val;
        let mut io2_1: *const TValue = &raw mut temp;
        (*io1_1).value_ = (*io2_1).value_;
        (*io1_1).tt_ = (*io2_1).tt_;
        from = from.offset(1);
        to = to.offset(-1);
    }
}

DEBUG:root:Running merge_rust with args: [PosixPath('/home/perl/Work/c2rust/tools/merge_rust/target/release/merge_rust'), '--update-only', PosixPath('repo/src/lapi.rs'), '/tmp/tmprchjvmuc']


thread 'main' (3666313) panicked at src/main.rs:38:44:
called `Result::unwrap()` on an `Err` value: At("parsing \"repo/src/lapi.rs\"", Syn(Error("cannot parse string into token stream")))
stack backtrace:
   0: __rustc::rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: merge_rust::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Using gemini-3.5-flash by the way.

Root cause: The broken block is in lua_gc. The match closes at tests/integration/tests/lua/repo/src/lapi.rs:2097, but the function itself does not close before the next #[no_mangle] at tests/integration/tests/lua/repo/src/lapi.rs:2099. A closing } is missing after return res;.

Potential solution:

  • check syntax using cargo check or rustc before attempting merge_rust and retry transform or
  • retry transform if merge_rust fails

@thedataking thedataking force-pushed the perl/postprocess-lua branch from b35967c to f4f226d Compare June 15, 2026 04:41
@thedataking thedataking changed the base branch from perl/postprocess-json-c-gpt-5.5-2026-04-23 to master June 15, 2026 04:43
@thedataking thedataking marked this pull request as ready for review June 15, 2026 04:43
@thedataking thedataking changed the base branch from master to perl/postprocess-json-c-gpt-5.5-2026-04-23 June 15, 2026 04:46
@thedataking thedataking force-pushed the perl/postprocess-json-c-gpt-5.5-2026-04-23 branch from 9d8d679 to 9a5f17f Compare June 15, 2026 04:47
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.
@thedataking thedataking force-pushed the perl/postprocess-lua branch from f4f226d to 7df7773 Compare June 15, 2026 04:51

@Crocodoctopus Crocodoctopus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks generally good, but I want to mention two concerns, mostly related to prompting:

I tested TrimTransform on json-c and it was very optimistic with the trim, with many false positives (probably 80% of comments). Double check if this is or isn't the case for lua.

Also on json-c, large functions can cause TrimTransform to fail in very unpredictable ways. I've caught TrimTransform refactoring entire functions. Even with prompt changes, this issue can persists.

json-c is outside the scope of this PR, so I wont block this on that front.

@thedataking

thedataking commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

I tested TrimTransform on json-c and it was very optimistic with the trim, with many false positives (probably 80% of comments). Double check if this is or isn't the case for lua.

This is a problem, this PR shouldn't regress json-c. Did you see the false positives with gemini-3.1-flash-lite or gemini-3.5-flash or something else?

@Crocodoctopus

Copy link
Copy Markdown
Contributor

gemini-3.1-flash-lite, gpt-5.5, and gpt-5.1 had this behavior. I haven't tried 3.5-flash.

Edit: Upon testing with 3.5-flash it looks correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants