Skip to content

Make compiler deterministic: use BTreeMap for property_analysis#11219

Merged
ogoffart merged 1 commit intoslint-ui:masterfrom
tilladam:fix/deterministic-binding-analysis
Apr 2, 2026
Merged

Make compiler deterministic: use BTreeMap for property_analysis#11219
ogoffart merged 1 commit intoslint-ui:masterfrom
tilladam:fix/deterministic-binding-analysis

Conversation

@tilladam
Copy link
Copy Markdown
Contributor

@tilladam tilladam commented Apr 1, 2026

Summary

  • Replace HashMap<SmolStr, PropertyAnalysis> with BTreeMap in Element::property_analysis to ensure deterministic iteration order
  • Update namedreference.rs to use btree_map::Entry instead of hash_map::Entry
  • Update move_declarations.rs to use BTreeMap for local property analysis collections

The HashMap was iterated in several passes (inlining, move_declarations, optimize_useless_rectangles, resolve_native_classes). Since HashMap uses Rust's default RandomState hasher, iteration order varied between runs, which is a source of non-determinism in the compiler.

This is a standalone improvement — a deterministic compiler is desirable regardless. It may or may not be sufficient to resolve the cross-platform diagnostic position differences seen in #10552; that would need to be validated by merging this and re-running CI on that branch.

Related: #11188

Test plan

  • cargo check -p slint-compiler passes
  • CI passes

Copy link
Copy Markdown
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

since visited is not iterated, i don't really understand where the non-determinism comes from.

Comment thread internal/compiler/namedreference.rs Outdated
Replace `HashMap<SmolStr, PropertyAnalysis>` with `BTreeMap` in
`Element::property_analysis` to ensure deterministic iteration order.

The HashMap was iterated in several passes (inlining, move_declarations,
optimize_useless_rectangles, resolve_native_classes), and since HashMap
uses Rust's default RandomState hasher, the iteration order varied
between runs. This non-determinism propagated through the compiler,
causing binding loop diagnostics to be emitted at different source
positions across runs/platforms.

Also updates namedreference.rs to use btree_map::Entry instead of
hash_map::Entry, and move_declarations.rs to use BTreeMap for the
local property_analysis collections.
@tilladam tilladam force-pushed the fix/deterministic-binding-analysis branch from b033bbf to 1ef8e9e Compare April 1, 2026 15:44
@tilladam tilladam changed the title Make binding analysis deterministic by using BTreeSet Make compiler deterministic: use BTreeMap for property_analysis Apr 1, 2026
Copy link
Copy Markdown
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

This makes sense.

@ogoffart ogoffart enabled auto-merge (squash) April 1, 2026 18:39
@ogoffart ogoffart merged commit 17981ae into slint-ui:master Apr 2, 2026
45 checks passed
tilladam added a commit to tilladam/slint that referenced this pull request Apr 10, 2026
Now that slint-ui#11219 (BTreeMap for property_analysis) is merged, the
compiler's analysis order is deterministic. Restore the three syntax
tests that were removed due to platform-dependent diagnostic positions:

- binding_loop_issue_7849.slint (regression test for slint-ui#7849)
- binding_loop_nested_repeater.slint
- binding_loop_root_changed_callback.slint

Also update loop descriptions in binding_loop_if_condition.slint and
binding_loop_init_callback.slint to reflect the new deterministic
traversal order which now includes the current property as the loop
starting point.
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