Refactor built-in function dispatch to enum switch#506
Open
bertysentry wants to merge 1 commit into
Open
Conversation
Replace the ~21-branch if/else-if chain in BuiltinFunctionCallAst.populateTuples(), which compared a stored int against repeated BUILTIN_FUNC_NAMES map lookups, with a BuiltinFunction enum resolved once in the constructor and dispatched via switch. The BUILTIN_FUNC_NAMES map of synthetic integer token values is removed: its values were never used, only name membership (lexer) and identity (dispatch), both now covered by the enum. Duplicated arity validation is factored into shared helpers; structurally unique builtins (sprintf, length, srand, sub/gsub, split, substr) get their own named methods. Behavior and error messages are preserved exactly: mvn verify shows zero per-test status changes across the 939 gawk/BWK/POSIX compatibility tests, and checkstyle/pmd/spotbugs report no findings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Fixes #505
What
Replaces the ~21-branch if/else-if chain in
BuiltinFunctionCallAst.populateTuples()(AwkParser.java) — which compared a storedintindex against repeatedBUILTIN_FUNC_NAMES.get(...)map lookups by string literal — with a table-driven, enum-based dispatch:BuiltinFunctionenum with one constant per AWK built-in (ATAN2(atan2)...TOUPPER(toupper)), each carrying its AWK source-level name, plus aBY_NAMEmap andBuiltinFunction.of(String)resolver. It replaces theMap<String, Integer>of synthetic token values and thefIdx = 257counter; those integer values were never used as values, only for name membership (lexer) and identity (dispatch).BuiltinFunction.of(text) != nullinstead of probing the map.BuiltinFunctionCallAstresolves its enum constant once in the constructor;populateTuples()is a flatswitch (builtin). Duplicated arity validation is factored into two shared helpers (populateArgumentsTuplesfor the fixed-arity math/string functions,populateOneArgumentTuplesforclose/tolower/toupper/system), and the structurally unique builtins (sprintf,length,srand,sub/gsub,split,substr) each get their own named, Javadoc''d private method. The unreachableNotImplementedErrorfallback is preserved as thedefaultcase.Behavior
No functional change intended. All error messages, argument-count checks, and tuple-emission order are preserved exactly, including existing quirks (e.g.
sub''s error messages being reused forgsub).Verification
mvn formatter:format: no changes needed.mvn verifyrun on the pristine tree (baseline) and with this change: both pass, and a per-test comparison oftarget/failsafe-reportsshows zero status changes across all 939 gawk/BWK/POSIX compatibility tests (453 passed / 253 skipped / 111 failed / 122 errors in both runs — the failures are pre-existing compatibility gaps, unchanged test-by-test).🤖 Generated with Claude Code