Skip to content

chore: add testcase for slice equality failing in comptime interpreter#5277

Closed
TomAFrench wants to merge 6 commits intomasterfrom
tf/slice-equality-failing-test
Closed

chore: add testcase for slice equality failing in comptime interpreter#5277
TomAFrench wants to merge 6 commits intomasterfrom
tf/slice-equality-failing-test

Conversation

@TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

This PR adds a test case for #5273 which tracks whether the compile is properly handling comptime slice equalities. Once #5273 is fixed then it should be moved to compile_success_empty.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@jfecher
Copy link
Contributor

jfecher commented Jun 18, 2024

This test needs to be exempted from the legacy tests for CI to pass.
I think we can just wait and add this test as part of #5276 when it is expected to work though.

Sorry, wrong PR. This is part of the much larger work on getting trait dispatch & operator overloading working in the interpreter

@TomAFrench
Copy link
Member Author

I've added an entry to ignore the legacy test.

TomAFrench and others added 4 commits June 18, 2024 18:32
@TomAFrench TomAFrench requested a review from a team June 20, 2024 10:51
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

I don't see why we're adding a failing test now rather than waiting until we add a PR to fix it to add the passing test like we usually do?

@TomAFrench
Copy link
Member Author

My thinking on this is it's generally just observability. Tests become us asserting that the compiler does what we expect rather than just "working correctly". This ensure that if the behaviour of the compiler changes in future then we'll be aware of it.

@jfecher
Copy link
Contributor

jfecher commented Jun 20, 2024

@TomAFrench I can see that argument, but doesn't it apply to all known issues? I think we should just wait until traits / operator overloading is implemented in the interpreter to add this test since I don't see it being accidentally added.

@TomAFrench TomAFrench closed this Jul 15, 2024
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