Skip to content

Add enumerate() to DSLX stdlib#3816

Closed
joannabrozek wants to merge 5 commits intogoogle:mainfrom
antmicro:std-enumarate
Closed

Add enumerate() to DSLX stdlib#3816
joannabrozek wants to merge 5 commits intogoogle:mainfrom
antmicro:std-enumarate

Conversation

@joannabrozek
Copy link
Copy Markdown
Contributor

This PR adds the 'enumerate()' function with the 'generics' attribute to DSLX stdlib, replacing the existing built-in implementation.

This change was created because we wanted to enable conversion 'enumerate()' to IR.

@joannabrozek joannabrozek changed the title Add enumerate() to DSLX stdlib WIP: Add enumerate() to DSLX stdlib Feb 10, 2026
@joannabrozek joannabrozek changed the title WIP: Add enumerate() to DSLX stdlib Add enumerate() to DSLX stdlib Feb 10, 2026
@joannabrozek joannabrozek marked this pull request as draft February 10, 2026 10:06
@joannabrozek
Copy link
Copy Markdown
Contributor Author

joannabrozek commented Feb 10, 2026

This PR is marked as draft since after adding function with the 'generics' attribute to stdlib, the xls_dslx_fmt_test test started failing, so we're looking into it. We suspect that this test doesn't support GenericTypeAnnotation.

@joannabrozek
Copy link
Copy Markdown
Contributor Author

This PR is marked as draft since after adding function with the 'generics' attribute to stdlib, the xls_dslx_fmt_test test started failing, so we're looking into it. We suspect that this test doesn't support GenericTypeAnnotation.

This issue has now been resolved, so this PR is ready for review.

@joannabrozek joannabrozek marked this pull request as ready for review February 10, 2026 14:19
@tmichalak
Copy link
Copy Markdown

@richmckeever PTAL

@joannabrozek
Copy link
Copy Markdown
Contributor Author

I rebased on the current main branch and applied suggested changes.
If there's anything else I need to do, let me know.

@rw1nkler
Copy link
Copy Markdown
Contributor

Do we need anything else here? @richmckeever

@joannabrozek
Copy link
Copy Markdown
Contributor Author

@richmckeever, I've noticed that the CI failed on internal checks. Can you help me find which test case fails?
Regarding the ZSTD pipeline, I think it should start passing after merging this PR: #3931

@joannabrozek
Copy link
Copy Markdown
Contributor Author

joannabrozek commented Mar 26, 2026

I've rebased changes onto the latest main, so fix for ZSTD pipeline from
#3931 is now included.

@kbieganski
Copy link
Copy Markdown
Contributor

@richmckeever Can we get a hint as to what internal checks are failing on this?

@richmckeever
Copy link
Copy Markdown
Collaborator

I think what we are seeing internally is just more cases where we would have to replace enumerate() with std::enumerate(). It may be easiest if you could separate the parts that are removing the old builtin into a separate PR. Then we can commit this PR, change all internal stuff to use the new function, and then you commit the removal of the old function.

@joannabrozek
Copy link
Copy Markdown
Contributor Author

@richmckeever, I split this PR into two:

Please let me know if there is anything else I can do for you.

@mikex-oss
Copy link
Copy Markdown
Collaborator

Closing in favor of #4021 and #4022.

@mikex-oss mikex-oss closed this Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants