refactor: dedupe and flatten the sprint's checker, emitter, and parser additions - #813
Merged
Conversation
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.
A readability pass over the code #804-#812 added, provably behavior-neutral: the refactored compiler emits byte-identical IR to the pre-refactor compiler across 552 corpus files, including the compiler's own 5.4 MB closure. Net -16 source lines.
checker
contains, plus a cross-registryhas_method_declarationread that was provably equivalent to a same-map lookup. It is now oneMap[String, Bool]keyed"<key>#<kind>"and a nameddup_check_applies— and the previously implicit third case (an unresolvable impl target is exempt) is stated in a comment.optional_plain_comparison's two mirrored halves and the tail ofargument_widens_to_optionalwere the same predicate written three times; extracted asoptional_accepts_plain.check_generic_enum_variant_construction: removed an unreachable E212 branch (the sole caller'senum_decl_has_variantguard is the gate — now said in a comment) and its now-unusedenum_nameparameter; replaced an inline copy ofargument_expression_indexwith the helper.normalized_method_nameretires two hand-rolled leading-dot strips.propagate_empty_argument_collection_type: a condition whose both arms reached the samereturnnow callscollection_literal_coercesfor its recording effect and says so.generic_enum_construction_basecall in the annotated-binding path.emitter
ir_emit_return_valueshed theif true:wrapper the extraction in fix: give a match on an rc-payload optional exact ownership #809 left behind (pure dedent).ir_bind_owned_rc_payload, carrying the full untracked-name rationale that previously lived only on the enum copy.ir_collect_string_bind_names, is one helper,ir_prepass_track_or_poison; the optional-match hook also gained a comment stating it sees only top-level arm patterns (unlike the pat_variant hook, which recurses).ir_plain_compare_operand_ok.ir_emit_optional_optional_compare/ir_emit_optional_plain_compareshared two verbatim blocks; nowir_emit_inner_value_eqandir_emit_guarded_eq.is_tuple_literal) and its comment now says explicitly that the second"tuple"is an AST node kind, not the rc kind spelled the same; the stale "therefore still leaks its box" sentence onir_owned_arg_kind_releasesis corrected to point at the caller that now handles it.ir_emit_match_arm_returndrops an unusedpuband its trailing flag-set is annotated as belt-and-braces.parser
parse_fn_declbuilt its child list, then rebuilt it around the where clause with an index walk and aret_nodesentinel ordering dance; it now builds the list once, after the clause. The bound-list grammar is oneparse_bound_listshared by the inline and where spellings so they cannot drift, and the merge is a namedmerge_generic_param_bounds.for g_idx, gifollows the value-then-index convention (gp_idx, gp_i), stops at the first match, andgenericsis declaredmut(it was written through an index while immutable — E216 only covers ident targets, which is itself worth a look someday).what was tested
make bootstrap-verify— fixed point holds aftermake refresh-bootstrap-seed(the seed diff is the recompiled refactored source itself, per the A/B above).make check-invalid-only(53 passed) — E262/E263/E264/E226 rejections all unchanged.make run-regressions-only(370 passed).