Implement gawk array sorting and type introspection builtins#504
Implement gawk array sorting and type introspection builtins#504bertysentry wants to merge 8 commits into
Conversation
Add GawkExtension, enabled by default, providing asort(), asorti(), typeof(), isarray(), mkbool(), and gensub(), with runtime support for PROCINFO["sorted_in"] traversal order, IGNORECASE-aware sorting, and gawk's typed/untyped value distinctions. Extension SPI additions: @JawkBeforeStart startup hook, @JawkRawValue and @JawkRegexp parameter annotations, method-level @JawkAssocArray positions, and a ForInKeyOrder hook so extensions control for-in traversal without gawk-specific code in AVM/JRT/AwkParser. Also fix regexp literal expression semantics (/re/ evaluates as $0 ~ /re/ per POSIX), accept extra user-function arguments with a gawk-style runtime warning (evaluated then discarded), route runtime warnings to stderr instead of the output sink, expose honest SYMTAB/FUNCTAB content, and keep extension state per-engine. Closes #454 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a8e20523b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (extensions == null || extensions.isEmpty()) { | ||
| return ExtensionSetup.EMPTY; | ||
| return ExtensionSetup.createDefault(); |
There was a problem hiding this comment.
When a Java caller passes an explicit empty extension list, this treats it the same as omitting extensions and installs GawkExtension anyway. That leaves no public Java API path for hosts to run with no extensions, so scripts that intentionally need to reclaim names such as gensub or typeof still see those names reserved even with new Awk(Collections.emptyList()) or an empty varargs call.
Useful? React with 👍 / 👎.
typeof()/isarray() on a missing array element now bring that element into existence (observable via `in`, delete, for-in), matching gawk. Achieved by reading array-element raw values through the normal element dereference, which already returns the untyped marker while creating the element; the special PEEK_ARRAY_ELEMENT_RAW opcode and JRT.peekAwkValue are removed as no longer needed. An explicit empty extension list (new Awk(emptyList) or empty varargs) is now honored as "no extensions" instead of silently installing the default set, giving Java hosts a way to reclaim names such as gensub or typeof. The no-argument constructors still install the default set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tics Extension SPI: - Add @JawkOptional for trailing optional parameters (null when omitted); asort/asorti/typeof/gensub now declare real signatures instead of varargs. @JawkAssocArray is parameter-level only again: the method-level indexed form is gone (indexes were unfriendly), and a pure-runtime Map check cannot work because a fresh dest variable must be compiled as an array reference. - Make StrNum public: it is the strnum type class and extensions need it. Diagnostics: - Replace the per-tuple runtimeWarning fields (CallFunctionTuple, ExtensionTuple) with a single WARNING opcode the parser plants before the instruction it describes. - Move the gensub third-argument warning out of the parser into GawkExtension itself, where it now checks the actual runtime value (variable selectors warn too, like gawk); AVM exposes the source description and current call line for extension diagnostics, and extension call sites are stamped with the call-site line. Core cleanups: - Rename AwkSettings.setAllowArraysOfArrays to setPosix (logic reversed): the flag now gates all POSIX compile-time behavior, including typed regexp literals. - Rename JRT.getAwkValue to getAssocArrayValue. - Single source of truth for special variable names: AVM exposes getSpecialVariableNames() backed by the same map getVariable() uses; GawkExtension and execMatch consume it. - HashAssocArray/SortedAssocArray normalize null values to the untyped marker so callers never special-case null. - VariableManager.getVariable is a plain abstract method (implemented in the JMH benchmark); sourceBasename uses File.getName; warnings use String.format; comparator helpers documented, including why they cannot reuse JRT.compare2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex Please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a92bcbfc10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ExtensionRegistry now stores factories instead of instances: resolve() and listExtensions() return a fresh instance for factory-registered extensions, so the stateful built-ins (GawkExtension, StdinExtension) are never shared across engines by programmatic callers. A new register(String, Supplier) overload lets third parties register factories for their own stateful extensions; register(String, instance) keeps its sharing semantics. The CLI's GawkExtension clone is obsolete. SYMTAB now observes initialized runtime globals: the SET_NUM_GLOBALS handler consumes the runtime-managed preamble tuples (ENVIRON, ARGC, ARGV offsets) before running the beforeStart hooks, so the gawk extension's SYMTAB snapshot sees their real values instead of untyped placeholders. The tuple stream stays the source of truth, which keeps sandboxed programs (which omit the ARGV tuple on purpose) intact. ARGC/ARGV also join AVM's special-variable map, answered through their lazy synthetic accessors, so SYMTAB["ARGV"][0] works in embedded runs where ARGV is never materialized. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex Please review again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6143c74242
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Stack before: ...<br/> | ||
| * Stack after: x ... | ||
| */ | ||
| PEEK_DEREFERENCE, |
There was a problem hiding this comment.
When an AwkExpression contains a raw-value extension call such as typeof(x) or isarray(x), the tuple stream uses this new PEEK_DEREFERENCE opcode instead of DEREFERENCE. The eval optimizer only keeps the SET_NUM_GLOBALS initializer for opcodes listed in requiresEvalGlobalFrame, and this opcode is not listed there, so it can remove the global-frame setup; executing the peek then reads runtimeStack.globals while it is still null. Mark this opcode as requiring an eval global frame, like DEREFERENCE, so eval of raw global variables does not fail before the extension runs.
Useful? React with 👍 / 👎.
| key = lKey; | ||
| } | ||
| result = BLANK; | ||
| result = UNTYPED; |
There was a problem hiding this comment.
Returning UNTYPED from a normal missing-element read leaks a subclass of UninitializedObject into the existing field-assignment path. With default arrays, BEGIN { $3 = a[1]; print NF } now goes down jrtSetInputField's uninitialized branch and does not create fields 1..3, whereas the previous blank value took the normal assignment path; the sorted-array implementation has the same change. Convert this marker before ordinary scalar/field assignment, or otherwise keep it out of non-introspection reads.
Useful? React with 👍 / 👎.
| Pattern pattern = regexp instanceof Pattern ? | ||
| (Pattern) regexp : Pattern.compile(toAwkString(regexp)); |
There was a problem hiding this comment.
When IGNORECASE is nonzero, gawk makes regexp operations case-insensitive (GNU awk manual), but gensub matches with the precompiled literal Pattern or a freshly compiled string pattern that has no CASE_INSENSITIVE flag. For example, after IGNORECASE = 1, gensub(/a/, "x", "g", "A") still leaves A unchanged. Rebuild or compile the pattern with the current IGNORECASE setting before creating the matcher.
Useful? React with 👍 / 👎.
Add PEEK_DEREFERENCE and EXTENSION to the opcodes requiring the eval global frame: an AwkExpression containing a raw-value extension call such as typeof(x) had its SET_NUM_GLOBALS initializer optimized away, crashing on the null globals array. Extension calls also read globals (e.g. IGNORECASE) and run beforeStart hooks, so they keep the frame too. gensub() now honors IGNORECASE: when it is nonzero, the pattern is recompiled with CASE_INSENSITIVE, matching gawk's rule that IGNORECASE affects all regexp operations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ENVIRON and ARGV preamble tuples are now emitted only when the script references them, or when it references SYMTAB (whose snapshot exposes them); unreferenced ones are answered by the synthetic accessors, so plain scripts no longer pay for building the environment map. ARGC stays always materialized: it is a single cheap assignment and its slot must remain authoritative for ARGC=n command-line operand assignments to affect input traversal. ARGV population derives its count from the argument list so it no longer depends on the ARGC slot. IGNORECASE is now a JRT-managed special variable like FS or RS: reads and writes compile to dedicated PUSH_IGNORECASE/ASSIGN_IGNORECASE opcodes, the JRT precomputes the boolean on assignment, and every regexp/sorting consumer tests jrt.isIgnoreCase() instead of coercing the value per match. -v/API assignments flow through the same setter. Wiring IGNORECASE surfaced a general parser hole: ++/-- on JRT-managed specials compiled to slot-based INC/DEC opcodes that never touched the JRT (gawk's aasort fixture loops on IGNORECASE++ and span forever). Pre/post increment and decrement of specials now compile to a read/adjust/assign sequence through the special-variable opcodes, with correct prefix/postfix value semantics. The duplicated special-variable push/assign switches are consolidated into single parser-level methods. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SYMTAB and FUNCTAB move from GawkExtension into the interpreter, where they belong: the parser emits UPDATE_SYMTAB/UPDATE_FUNCTAB preamble tuples only when the script references them outside POSIX mode, and the AVM populates them from its own metadata (script globals, JRT-managed specials, host-supplied variables, function names, extension keywords). This fixes gawk parity for the reported case: -v variables without a compiled slot now appear in SYMTAB, and command-line name=value operand assignments update the array live between input files. The extension beforeStart hooks now run from a dedicated BEFORE_START_HOOKS opcode emitted by the parser at the end of the preamble, replacing the AVM's nested preamble-consumption loop, and execute at most once per AVM: reused interpreters (repeated executions, expression evaluations) no longer reinitialize their extensions. The special-variables accessor table is now a static Map<String, Function<AVM, Object>>, so constructing an AVM allocates nothing for it. GawkExtension loses all SYMTAB/FUNCTAB code (its beforeStart hook only registers the for-in ordering), which also retires the shouldMaterialize/getVariable lookup concern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AVM.getVariable(String) is back to a plain string switch (a compiled jump table, no map allocation and no lookup indirection); the special variable names live in a static Set that a unit test keeps in sync with the switch. @unsorted now performs no sorting at all: asort()/asorti() skip Collections.sort entirely (the for-in hook already returned keySet() directly), and the constant-comparator branch is gone. JRT.toAssignedScalar is renamed untypedToBlank with documentation of where the untyped marker comes from (missing-array-element reads) and why assignments must strip it (x = a[missing] must leave x an assigned blank scalar, as in gawk). The pointless call on a freshly boxed Integer in populateArgc is removed; the two real call sites remain, costing one monomorphic instanceof on the assignment paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Closes #454
What changed
GawkExtension, enabled by default (innew Awk()and the CLI when no-lis given), implementing:asort(source [, dest [, how]])/asorti(...)— gawk array sorting with the predefined comparison modes (@ind_str_asc,@val_num_desc,@val_type_asc,@unsorted, …), honoringIGNORECASEtypeof(x)— gawk type categories (number,string,strnum,array,regexp,number|bool,unassigned,untyped)isarray(x),mkbool(x), andgensub(re, repl, how [, target])(gawk semantics: anyg/G-prefixed selector is global,\Nbackreferences)PROCINFO["sorted_in"]-controlledfor (i in a)traversal, re-read at each loop so mid-execution changes applySYMTAB(script globals + Jawk special variables, snapshot) andFUNCTAB(user-defined functions + extension keywords) for scripts that reference themExtension SPI — gawk-specific behavior lives entirely in the extension; the core gained neutral hooks:
@JawkBeforeStart— setup method run after globals are allocated, before execution@JawkRawValue— parameter receives the runtime value uncoerced (howtypeof/isarraysee untyped state); backed by newPEEK_DEREFERENCE/PEEK_ARRAY_ELEMENT_RAWopcodes that observe without autovivifying@JawkRegexp— parameter keeps regexp literals as precompiled patterns@JawkAssocArray({...})for array positions consumed through varargsForInKeyOrder— extension-providedfor-intraversal order; the default path is unchanged (singlekeySet()copy, null-check only)Interpreter fixes riding along:
$0 ~ /re/per POSIX (x = /re/yields 0/1)@/re/) accepted; rejected under--posixerrorStream(...)captures it in the Java API)AVM.getVariable(name)falls back to-v/API-supplied initial variables, so e.g.-v IGNORECASE=1is honored even when the script never references itsub()/gsub()replacement escaping is shared withgensubviaRegexRuntimeSupportReviewer notes
GawkExtensionperAwk; the singletonINSTANCEpattern was removed becauseAbstractExtensionholds per-enginevm/jrtbindings.ExtensionFunctionistransientand recomputed from the resolvedMethodon deserialization, so.tuplesfiles from other versions stay loadable.VariableManager.getVariable(String)is adefaultmethod to keep third-party implementors source- and binary-compatible.GawkExtensionIT.test_asortsymtabgoes from pass to failure. Its.okfixture only encodes the entry counts of the gawk build's own symbol table (30 globals / 42 builtins); the previous implementation faked those counts with placeholder entries, which corrupted any script actually readingSYMTAB/FUNCTAB. The honest content can't match gawk's internal table sizes.index.md.vm(gawk departures list),extensions.md,extensions-writing.md,cli.md,cli-reference.md, andjava.md— including thatgensub/typeof/asort/… become reserved names by default.Verification
mvn verify: 479 unit tests, 0 failures; checkstyle, PMD, SpotBugs clean;mvn compile -P benchmarkpassesmainbaseline: 29 cases error→pass (incl. all 13 representative cases from Implement gawk array sorting and type-introspection builtins #454), 10 error→failure (now run, output differs — follow-up candidates), 1 pass→failure (asortsymtab, intentional, see above), zero other regressions across POSIX/BWK/gawk suites🤖 Generated with Claude Code