Skip to content

Match element fixes - #13002

Merged
0x6e merged 2 commits into
slint-ui:masterfrom
R-Cramer4:match-fixes
Sep 18, 2026
Merged

Match element fixes#13002
0x6e merged 2 commits into
slint-ui:masterfrom
R-Cramer4:match-fixes

Conversation

@R-Cramer4

@R-Cramer4 R-Cramer4 commented Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This pr resolves the rest of the issues in #12680

  • Matching on floats now produces an error
    • It also errors on float to int conversion except in the case of a float like 1.0 or 3.0
  • No more O(N2) operations while checking the cases for duplicates/exhaustiveness
    • This did result in Hash being implemented for EnumerationValue and derived for Unit
  • Added js test blocks for all match statement tests
  • Duration with different units test case was added already
  • Nit about potentially unnecessary use of Option I believe was already addressed

0x6e
0x6e reviewed Aug 24, 2026
View reviewed changes
Comment thread internal/compiler/tests/syntax/match_element/float_error.slint
0x6e
0x6e reviewed Aug 24, 2026
View reviewed changes
Comment thread internal/compiler/tests/syntax/match_element/float_error.slint
@R-Cramer4 R-Cramer4 changed the title Match statement fixes Match element fixes Aug 24, 2026
0x6e
0x6e reviewed Aug 24, 2026
View reviewed changes
Comment thread internal/compiler/passes/resolving.rs
0x6e
0x6e reviewed Aug 24, 2026
View reviewed changes
Comment thread internal/compiler/passes/resolving.rs Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
0x6e
0x6e reviewed Aug 27, 2026
View reviewed changes
Comment thread internal/compiler/tests/syntax/match_element/float_error.slint Outdated
Comment thread internal/compiler/tests/syntax/match_element/float_error.slint
Comment thread internal/compiler/passes/resolving.rs Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
0x6e
0x6e reviewed Aug 28, 2026
View reviewed changes
Comment thread internal/compiler/tests/syntax/match_element/cast.slint Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
R-Cramer4
R-Cramer4 commented Aug 28, 2026
View reviewed changes
Comment thread internal/compiler/passes/resolving.rs Outdated
0x6e
0x6e reviewed Sep 8, 2026
View reviewed changes
Comment thread internal/compiler/tests/syntax/match_element/cast.slint Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
Comment thread internal/compiler/tests/syntax/match_element/cast.slint
0x6e
0x6e reviewed Sep 8, 2026
View reviewed changes
Comment thread internal/compiler/passes/resolving.rs Outdated
@0x6e

0x6e commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks, I think this is ready for a final ok from @ogoffart .

@0x6e
0x6e requested a review from ogoffart September 9, 2026 08:58
@0x6e 0x6e mentioned this pull request Sep 9, 2026
Open
13 tasks
ogoffart
ogoffart reviewed Sep 9, 2026
View reviewed changes

@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.

I'm not sure about this float restriction.
We allow == on float.
matching a float on a 0 is the same as matching it on a 0.0
Also there is no restruction for, say, length which has the same problem.

Comment thread internal/compiler/langtype.rs
Comment thread internal/compiler/tests/syntax/match_element/float_error.slint Outdated
Comment thread internal/compiler/tests/syntax/match_element/float_error.slint Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
Comment thread internal/compiler/passes/resolving.rs Outdated
@0x6e

0x6e commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

The float restriction was based on this. If you think we should allow it, I think it would have to be allowed based on the existing fuzzy comparison in Slint.

We have to consider how we want to handle cases that would fuzzy-compare as equal, resulting in two elements being permitted through the match element instead of one. That would be no different from two conditional elements, but might not be the expected behaviour of a match element.

We could potentially allow it with a warning?

0x6e
0x6e reviewed Sep 11, 2026
View reviewed changes
Comment thread internal/compiler/tests/syntax/match_element/float_error.slint Outdated
@R-Cramer4 R-Cramer4 mentioned this pull request Sep 14, 2026
Draft
@R-Cramer4

R-Cramer4 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

I pulled the float error code into its own PR (#13399) so we can decide on that later. This PR now just has the two commits which hopefully can be merged.

@R-Cramer4
compiler: Removed O(N^2) operations in match statements
cca8903 
Hash values when checking for duplicates/exhaustiveness instead of
running a O(N^2) search over them
@R-Cramer4
tests: Added NodeJS match test cases
3469d6f 
Match statements are now covered in the NodeJS test driver as well
@0x6e
0x6e merged commit 62cdfb8 into slint-ui:master Sep 18, 2026
53 checks passed
@R-Cramer4
R-Cramer4 deleted the match-fixes branch September 18, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ogoffart ogoffart ogoffart left review comments
@0x6e 0x6e 0x6e approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@R-Cramer4 @0x6e @ogoffart