Skip to content

refactor: Account for idents in function signatures in reorganize_definitions#1859

Merged
thedataking merged 1 commit into
masterfrom
legare/libgit2-refactor-bug
Jul 9, 2026
Merged

refactor: Account for idents in function signatures in reorganize_definitions#1859
thedataking merged 1 commit into
masterfrom
legare/libgit2-refactor-bug

Conversation

@randomPoison

@randomPoison randomPoison commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

libgit2 surfaced a bug in reorganize-definitions where if a type was referenced only in a functions's signature (but not its body), reorganize-definitions would fail to emit an import for that type. The fix seems simple, just account for types used by the function signature.

@thedataking thedataking force-pushed the legare/libgit2-refactor-bug branch from 3e58e59 to cff2ba5 Compare June 17, 2026 03:36
used_idents.insert(segment.ident);
}
});
collect_used_idents(item);

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.

This is an extra refactoring that wasn't really necessary, I think the fix could have been one line. I don't hate the new code though, so let's land it as is.

@thedataking thedataking marked this pull request as ready for review June 18, 2026 00:22

@thedataking thedataking 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.

I don't hate the new code though, so let's land it as is.

I agree. This looks reasonable to me too. Removing draft status and updating PR body accordingly.

@thedataking thedataking added the bug Something isn't working label Jun 18, 2026
@thedataking thedataking changed the base branch from master to legare/module-name-collision June 18, 2026 09:34
@ahomescu

Copy link
Copy Markdown
Contributor

Is this ready to land?

spernsteiner added a commit to GaloisInc/Tractor-Crisp that referenced this pull request Jun 29, 2026
Cherry-picks the fix from immunant/c2rust#1859
(which is not yet merged)
@thedataking thedataking force-pushed the legare/module-name-collision branch 4 times, most recently from 9cb12d3 to 9f1dd8e Compare July 5, 2026 07:18
spernsteiner added a commit to GaloisInc/Tractor-Crisp that referenced this pull request Jul 8, 2026
Cherry-picks the fix from immunant/c2rust#1859
(which is not yet merged)
@thedataking thedataking force-pushed the legare/libgit2-refactor-bug branch from cff2ba5 to 3749f92 Compare July 8, 2026 23:07
@thedataking thedataking changed the base branch from legare/module-name-collision to master July 8, 2026 23:08
…finitions`

Previously only idents used in the function body were accounted for, which led to errors if there was a type referenced in the signature but not the body.
@thedataking thedataking force-pushed the legare/libgit2-refactor-bug branch from 3749f92 to 35550d0 Compare July 8, 2026 23:12
@thedataking thedataking changed the title Fix libgit2 reorganize-definitions bug refactor: Account for idents in function signatures in reorganize_definitions Jul 8, 2026
@thedataking thedataking merged commit 71d102c into master Jul 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants