Skip to content

Conversation

@david-pl
Copy link
Contributor

@david-pl david-pl commented Nov 3, 2025

This was removed as part of #344, so I'm not sure if it should be added back. However, without this change we always get IList[AnyType] unless explicitly specifying the element type in ilist.New. This breaks a bunch of tests (and code, really) in bloqade-circuit. Adding this back seemed like the much easier change than explicitly setting the elem_type everywhere.

@david-pl david-pl requested a review from Roger-luo November 3, 2025 09:42
@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/kirin/dialects/ilist/stmts.py 80.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
11480 10219 89% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/kirin/dialects/ilist/stmts.py 99% 🟢
TOTAL 99% 🟢

updated for commit: 77fd133 by action🐍

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

PR Preview Action v1.6.2
Preview removed because the pull request was closed.
2025-11-06 15:42 UTC

if not values:
elem_type = types.Any
else:
elem_type = values[0].type
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not correct? the type should be union of all elements being used to construct the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, my bad. Thanks for fixing it!

@Roger-luo Roger-luo merged commit 1dfd6e3 into main Nov 6, 2025
12 of 13 checks passed
@Roger-luo Roger-luo deleted the david/fix-ilist-elemtype branch November 6, 2025 15:42
david-pl added a commit to QuEraComputing/bloqade-circuit that referenced this pull request Nov 7, 2025
CI is blocked by:

* Requires kirin main
* This kirin PR: QuEraComputing/kirin#563
* This kirin issue: QuEraComputing/kirin#564

Other than that, we should be good. I'm still targeting the kirin
upgrade branch to make review easier, but this once the above issues are
resolved, this can actually go into `main`.

@weinbe58 please have a look at the address analysis, specifically at
the `run_lattice` method. I had to change the signature a bit.

---------

Co-authored-by: kaihsin <[email protected]>
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.

3 participants