Skip to content

Conversation

kevinhartman
Copy link
Contributor

Summary

Originally, I thought it'd make the most sense to reject construction of operations that'd have a duration type but also be non-const expressions. But, a user can currently create a Var with a duration type, so this seems like an incomplete restriction.

Details and comments

Consider this PR more of a suggestion—perhaps it makes more sense to also block construction of Var when the type is Duration. Either way, I think we should decide on this before 2.0.0.

Originally, I thought it'd make the most sense to reject
construction of operations that'd have a duration type
but also be non-const expressions. But, a user can
currently create a `Var` with a duration type, so this
seems like an incomplete restriction.
@kevinhartman kevinhartman added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Mar 12, 2025
@kevinhartman kevinhartman added this to the 2.0.0 milestone Mar 12, 2025
@kevinhartman kevinhartman requested a review from a team as a code owner March 12, 2025 21:03
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@coveralls
Copy link

Pull Request Test Coverage Report for Build 13821481747

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.01%) to 88.133%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 2 93.23%
Totals Coverage Status
Change from base Build 13819553093: 0.01%
Covered Lines: 72684
Relevant Lines: 82471

💛 - Coveralls

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

I'm fine with this; I don't think there's anything necessarily inherent to the duration type that requires it to be a compile-time constant forever. We can reject it in circuit-level type-checking if we want to.

@jakelishman jakelishman added this pull request to the merge queue Mar 27, 2025
Merged via the queue into Qiskit:main with commit bfa2331 Mar 27, 2025
21 checks passed
mergify bot pushed a commit that referenced this pull request Mar 27, 2025
Originally, I thought it'd make the most sense to reject
construction of operations that'd have a duration type
but also be non-const expressions. But, a user can
currently create a `Var` with a duration type, so this
seems like an incomplete restriction.

(cherry picked from commit bfa2331)
github-merge-queue bot pushed a commit that referenced this pull request Mar 27, 2025
Originally, I thought it'd make the most sense to reject
construction of operations that'd have a duration type
but also be non-const expressions. But, a user can
currently create a `Var` with a duration type, so this
seems like an incomplete restriction.

(cherry picked from commit bfa2331)

Co-authored-by: Kevin Hartman <kevin@hart.mn>
@ElePT ElePT added the Changelog: None Do not include in changelog label Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants