Skip to content

Add stable array.index-of, desugaring to find-index - #12791

Open
tilladam wants to merge 3 commits into
slint-ui:masterfrom
tilladam:array-index-of
Open

Add stable array.index-of, desugaring to find-index#12791
tilladam wants to merge 3 commits into
slint-ui:masterfrom
tilladam:array-index-of

Conversation

@tilladam

@tilladam tilladam commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #12790 (array.find-index) — this branch contains that commit plus one new one on top. The commit history is intentional: please review it commit-by-commit via the "Commits" tab, or wait for #12790 to merge and this will show a clean single-commit diff against master.

array.index-of(value) reads the index of the first element equal to value, or -1. Unlike find-index/any/all it takes a plain value, not a predicate closure, so the caller never writes closure syntax to use it.

Implemented as a compiler-side desugaring (array_index_of_macro, next to the existing push/remove/insert macros): it synthesizes an equality closure and a FunctionCall to the existing ArrayFindIndex builtin, then lowers exactly like a hand-written find-index((x) => x == value) call. No new codegen, no new interpreter arm, no new runtime function — it reuses model_find_index end to end.

value is evaluated exactly once, into a local variable read from inside the synthesized closure, rather than re-embedded in the closure body — the latter would re-run it once per row visited (and not at all against an empty array), breaking normal once-per-call argument evaluation for any value with a side effect or a non-deterministic result. Regression test: index-of-value-with-side-effect in tests/cases/models/array.slint.

Because closures are never part of the user-visible syntax for index-of, its lookup.rs entry sits outside the enable_experimental gate that guards any/all/find-index (verified by a dedicated test, index_of_stability.rs, since the syntax-test corpus and every runtime test driver force enable_experimental=true uniformly and so can't catch a regression here). index-of ships as a stable, ordinary array method documented alongside push/remove/insert/length, not as part of the experimental array-predicates guide.

Part 2 of 3:

  1. Add experimental array.find-index predicate #12790array.find-index, the experimental predicate this desugars to.
  2. This PRarray.index-of.
  3. ComboBox: reverse-lookup current-value writes into current-index #12792 — wires ComboBox's current-value writes to index-of, closing ComboBox and RadioGroup: writes to current-value are silently overridden #11970.

Test plan

  • cargo test -p i-slint-compiler --features display-diagnostics --test syntax_tests
  • cargo test -p i-slint-compiler --test index_of_stability
  • SLINT_TEST_FILTER=models/array cargo test --manifest-path tests/Cargo.toml -p test-driver-{interpreter,rust,cpp,nodejs}

@ogoffart ogoffart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One should also have a test in which the argument has a side effect.

in-out property <int> count;
function foo() { count += 1; return 42; }
init => { 
  test = [40, 41, 42, 43].index-of(foo()) == 3 && count == 1;
}
out property <bool> test;

We need to check that foo() is called only once.

Any other `index` leaves the array unchanged.
- `array.insert(index, value)` accepts `0 <= index <= array.length`, so inserting at `array.length` appends.
Any other `index` leaves the array unchanged.
- `array.index-of(value)` returns `-1` when `value` isn't found, including on an empty array.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not an "Out-of-bounds behavior".

Suggested change
- `array.index-of(value)` returns `-1` when `value` isn't found, including on an empty array.

-1 behavior is already specified previously.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, removed — that line was redundant with the -1 behavior already stated in the Operations section above. Pushed.

- `array.remove(index)` removes the element at `index`.
- `array.insert(index, value)` inserts `value` before the element at `index`, shifting later elements up.
`value` must have the element type.
- `array.index-of(value)` reads the index of the first element equal to `value` as an `int`,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `array.index-of(value)` reads the index of the first element equal to `value` as an `int`,
- `array.index-of(value)` returns the index of the first element equal to `value` as an `int`,

feels more natural to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied, thanks — "returns" also matches the verb pattern the other call-shaped operations already use (push "appends", remove "removes", insert "inserts"); "reads" was only right for the bare .length property. Pushed.

Comment thread internal/compiler/lookup.rs Outdated
Comment on lines +1220 to +1221
// `index-of` desugars to `find-index` (see `array_index_of_macro`) but never
// exposes closure syntax to the caller, so it isn't gated like `find-index` is.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// `index-of` desugars to `find-index` (see `array_index_of_macro`) but never
// exposes closure syntax to the caller, so it isn't gated like `find-index` is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. Pushed.

@tilladam

tilladam commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

One should also have a test in which the argument has a side effect... We need to check that foo() is called only once.

Already covered — index-of-value-with-side-effect / index-of-value-calls in tests/cases/models/array.slint:

in-out property<int> index-of-value-calls: 0;
public function index-of-value-with-side-effect() -> int {
    self.index-of-value-calls += 1;
    return 3;
}
public function index-of-with-side-effect() -> int {
    return self.ints.index-of(self.index-of-value-with-side-effect());
}

asserted across all three runtime drivers as index_of_with_side_effect() == 2 && index_of_value_calls == 1. This one actually caught a real bug during development — the first implementation embedded the value expression directly inside the synthesized predicate closure, so it ran once per row instead of once (see array_index_of_macro in builtin_macros.rs for the fix and the comment explaining why).

The other three suggestions are applied and pushed — replied inline on each.

Sibling to the array.any/array.all predicates (slint-ui#11989): returns the index
of the first element for which the closure holds, or -1 if none match.
Mirrors any/all end-to-end — parser/lookup, resolving, Rust and C++
codegen, api/cpp/include/private/slint_models.h, the interpreter, and
core::model::model_find_index — reusing the existing Expression::Closure/
Type::Closure plumbing. Same experimental gate as any/all (tracked by
slint-ui#12777); only usable inline, not stored or passed around, for the same
reasons any/all are restricted that way.

The interpreter reuses core's model_any/model_all/model_find_index for
the row iteration and dependency tracking, rather than open-coding the
loop a second time; a small eval_array_row_predicate helper does the
per-row work shared by any/all/find-index (bind arg_name, evaluate the
closure expression, restore the shadowed local var).

Added as groundwork for a real fix to slint-ui#11970 (ComboBox/RadioGroup
current-value writes), which needs a value-to-index reverse lookup that
plain any/all can't express.

changelog: Added an experimental `array.find-index((name) => condition)` predicate, returning the index of the first matching element or -1.
`array.index-of(value)` reads the index of the first element equal to
value, or -1. Unlike find-index/any/all it takes a plain value, not a
predicate closure, so the caller never writes closure syntax to use it.

Implemented as a compiler-side desugaring (array_index_of_macro, next to
the existing push/remove/insert macros): it synthesizes an equality
closure and a FunctionCall to the existing ArrayFindIndex builtin, then
lowers exactly like a hand-written `find-index((x) => x == value)` call.
No new codegen, no new interpreter arm, no new runtime function — it
reuses model_find_index end to end.

`value` is evaluated exactly once, into a local variable read from
inside the synthesized closure, rather than re-embedded in the closure
body — the latter would re-run it once per row visited (and not at all
against an empty array), breaking normal once-per-call argument
evaluation for any `value` with a side effect or a non-deterministic
result. Regression test: index-of-value-with-side-effect in
tests/cases/models/array.slint.

Because closures are never part of the user-visible syntax for index-of,
its lookup.rs entry sits outside the `enable_experimental` gate that
guards any/all/find-index (verified by a dedicated test:
index_of_stability.rs, since the syntax-test corpus and every runtime
test driver force enable_experimental=true uniformly and so can't catch
a regression here). index-of ships as a stable, ordinary array method
documented alongside push/remove/insert/length, not as part of the
experimental array-predicates guide.

changelog: Added a stable `array.index-of(value)` array method, returning the index of the first matching element or -1.
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.

2 participants