Skip to content

[bugfix] Drop functionsOrdered argument from AbstractInternalModule constructor#3

Merged
duncdrum merged 1 commit into
masterfrom
bugfix/6378-drop-ordered-flag
May 21, 2026
Merged

[bugfix] Drop functionsOrdered argument from AbstractInternalModule constructor#3
duncdrum merged 1 commit into
masterfrom
bugfix/6378-drop-ordered-flag

Conversation

@joewiz
Copy link
Copy Markdown
Member

@joewiz joewiz commented May 18, 2026

Companion to eXist-db/exist#6384 (closes eXist-db/exist#6378). That PR removes the 3-arg AbstractInternalModule(FunctionDef[], Map, boolean) constructor — the base class now always sorts the function table and always binary-searches, so the third argument has no remaining purpose. The flag was the root cause of #6376 (silent function-lookup failure when the FunctionDef[] wasn't sorted).

This one-line change drops the now-removed third argument so the build continues to compile once eXist#6384 merges.

No behavior change at the module level — the previous true value was either redundant (true → array was sorted anyway via the static sort idiom, or sort+binary was unnecessary at this module's scale) or implicit in the new always-sort behavior (false → linear scan and always-binary-search are equivalent for correctness; both find the function). The function lookup contract is unchanged.

[This response was co-authored with Claude Code. -Joe]

…onstructor

eXist-db/exist#6378 (PR eXist-db/exist#6384) removes the 3-arg
`AbstractInternalModule(FunctionDef[], Map, boolean)` constructor.
The 2-arg form is now the only path; the base class always sorts
the function table and always binary-searches.

This commit drops the now-removed third argument.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joewiz
Copy link
Copy Markdown
Member Author

joewiz commented May 20, 2026

[This response was co-authored with Claude Code. -Joe]

Status update: this PR is now optional cleanup rather than required.

Per @line-o's review on eXist-db/exist#6384, the 3-arg AbstractInternalModule(FunctionDef[], Map, boolean) constructor is reinstated as @Deprecated(since = "7.0.0", forRemoval = true) rather than removed. The functionsOrdered parameter is ignored (the base class always sorts), but legacy call sites continue to compile against eXist 7 with just a deprecation warning. So this repo would now build cleanly against develop without any source changes here.

The PR is still worth merging when convenient — dropping the unused third argument removes the warning and is the conventional Java forward path — but it's no longer time-sensitive. Feel free to merge or defer.

@duncdrum duncdrum merged commit bde63e8 into master May 21, 2026
2 checks passed
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.

AbstractInternalModule: functionsOrdered=true invariant is trusted but not enforced; silent function-lookup failure if array is unsorted

2 participants