refactor(hir): remove dead scaffolding and dedup scope queries#291
Merged
Conversation
DefKind declared 11 variants that DefId::kind never produces: Class, Covergroup, Checker, Modport, ClockingBlock, Sequence, Property, ClassField, Method, Struct, Enum. They only appeared in match arms returning Unknown/empty, which reads as 'intentionally unsupported' when it is actually 'not yet implemented'. With #[non_exhaustive] on DefKind, variants can be added together with their lowering later and the compiler will force every in-crate match to be updated then; there is no benefit to pre-declaring them now.
Ty::ClassHandle and Ty::VirtualInterface are never constructed anywhere in the crate; they only appear in match arms returning Unknown/empty, alongside the SpecArg enum that exists solely to feed ClassHandle. Class/interface specialization is deferred per the PR notes, so these are forward scaffolding that reads as implemented-but- empty. Drop them now and reintroduce with the class lowering later.
module/generate_block/block/subroutine scope queries each repeated the same decl, typedef, and stmt-label insertion loops. Extracting them into insert_decls_and_typedefs and insert_stmts makes a single source of truth so a new declaration category is added in one place, not four. Insertion order is preserved: the two helpers are called around the container- specific members (instances, generate children, ports).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cleanup follow-up to the merged semantic-foundations stack (#289/#290/#288), addressing the maintainability issues raised in review. Three independent commits, each behavior-preserving.
1. Drop dead
DefKindvariantsDefId::kind()never produced 11 variants (Class,Covergroup,Checker,Modport,ClockingBlock,Sequence,Property,ClassField,Method,Struct,Enum); they only sat in match arms returningUnknown/empty, which reads as "intentionally unsupported" when it is actually "not yet implemented".DefKindis already#[non_exhaustive], so variants can be reintroduced together with their lowering later and the compiler will force every in-crate match to update then.2. Drop dead
Tyvariants andSpecArgTy::ClassHandleandTy::VirtualInterfaceare never constructed anywhere; they only appear in match arms returningUnknown/empty, alongside theSpecArgenum that exists solely to feedClassHandle. Class/interface specialization is deferred, so these are forward scaffolding.3. Deduplicate scope query member insertion
module/generate_block/block/subroutinescope queries each repeated the same decl/typedef/stmt-label insertion loops. Extracted intoinsert_decls_and_typedefsandinsert_stmtsso a new declaration category is added in one place, not four. Insertion order preserved (the two helpers bracket the container-specific members).Validation
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace(483 passed, unchanged from baseline)Notes
Each commit passes the full gate independently. Deliberately left untouched: the unreachable
BuiltinDataTy::Event|Chandle|Voidarms inside theTy::Builtinmatches oftype_class/packed_bit_width— those types are promoted out ofTy::Builtinduring normalization, so the arms are unreachable, but removing them would need a_wildcard (losing exhaustive coverage ofBuiltinDataTy) or a larger representation unification; both are poorer tradeoffs than leaving the defensive arms. Flagging it here so it is a conscious decision, not an oversight.