Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Stretch] Don't reject non-const duration in constructors. #14009

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants