Skip to content

Conversation

@Scheremo
Copy link
Contributor

@Scheremo Scheremo commented Oct 30, 2025

This commit adds the plumbing to convert slang::ast::MethodPrototypeSymbols corresponding to class method forward declarations. This is done by looking up the implementation and converting it when encountering a forward declaration. This makes sure field order within the class declaration is respected and the emitted methoddecl's signature is correct.

It also fixes a bug where the implicit this reference argument of method calls would get emitted twice in call sites.

@Scheremo Scheremo marked this pull request as ready for review October 30, 2025 08:29
@Scheremo Scheremo changed the title [Moore][ImportVerilog] Add support for class method forward decls [ImportVerilog] Add support for class method forward decls Oct 30, 2025
@Scheremo Scheremo changed the title [ImportVerilog] Add support for class method forward decls [Bug][ImportVerilog] [1/2] Add support for class method forward decls Oct 30, 2025
@Scheremo Scheremo force-pushed the pr-class-method-forward-decl branch from e460cdf to c91f703 Compare October 30, 2025 15:17
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

LGTM, modulo minor nits

This commit adds the plumbing to convert `slang::ast::MethodPrototypeSymbol`s corresponding to class
method forward declarations. This is done by looking up the implementation and converting it when
encountering a forward declaration. This makes sure field order within the class declaration is
respected and the emitted `methoddecl`'s signature is correct.
@Scheremo Scheremo force-pushed the pr-class-method-forward-decl branch from c91f703 to 6a0fde0 Compare October 31, 2025 06:29
@Scheremo Scheremo merged commit 921f526 into main Oct 31, 2025
7 checks passed
@Scheremo Scheremo deleted the pr-class-method-forward-decl branch October 31, 2025 06:52
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.

3 participants