This repository was archived by the owner on Jan 12, 2024. It is now read-only.
Fix Unit Test Failures for Classical Control#289
Merged
ScottCarda-MS merged 25 commits intofeatures/ClassicallyControlledfrom Feb 7, 2020
Merged
Fix Unit Test Failures for Classical Control#289ScottCarda-MS merged 25 commits intofeatures/ClassicallyControlledfrom
ScottCarda-MS merged 25 commits intofeatures/ClassicallyControlledfrom
Conversation
Will now *not* hoist or convert anything in functions.
…lt expression to Zero.
ScottCarda-MS
requested review from
AaronRM,
bamarsha and
bettinaheim
and removed request for
AaronRM
…nts to make finding specific tests easier.
…arp-compiler into sccarda/HoistFixes
…et converted to ApplyIf calls.
Contributor
|
@ScottCarda-MS I would suggest to make this a PR into sccarda/ClassicallyControlledUnitTests. Then the proper diff is visible. |
bettinaheim
reviewed
Feb 2, 2020
Contributor
bettinaheim
left a comment
bettinaheim
left a comment
There was a problem hiding this comment.
Looks good! I love the tests - you did a really good job at composing a good set of unit tests!
src/QsCompiler/Tests.Compiler/TestCases/LinkingTests/ClassicalControl.qs
Outdated
Show resolved
Hide resolved
src/QsCompiler/Tests.Compiler/TestCases/LinkingTests/ClassicalControl.qs
Show resolved
Hide resolved
src/QsCompiler/Tests.Compiler/TestCases/LinkingTests/ClassicalControl.qs
Show resolved
Hide resolved
src/QsCompiler/Transformations/ClassicallyControlledTransformation.cs
Outdated
Show resolved
Hide resolved
src/QsCompiler/Transformations/ClassicallyControlledTransformation.cs
Outdated
Show resolved
Hide resolved
src/QsCompiler/Transformations/ClassicallyControlledTransformation.cs
Outdated
Show resolved
Hide resolved
bettinaheim
reviewed
Feb 3, 2020
…king on the controlled characteristic.
… Added four new tests.
bettinaheim
approved these changes
Feb 7, 2020
Contributor
bettinaheim
left a comment
bettinaheim
left a comment
There was a problem hiding this comment.
As discussed, approving for the merge into the feature branch despite the two disabled tests. These will be reenabled after merging this, then merging the updates from master, and getting a runtime package with both changes in it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR relies on PR #287.
Fixes an issue brought to light by the Classical Control unit test #9, where the conversion from an if/else statement to an ApplyIfElse statement assumed the condition compared against a result literal Zero.
Prevent Functions from being affected by the transformation.
Allow if blocks with a single non-call statement in them to be hoisted into their own operations.
Prevent hoisting of part of an if-structure.
Added feature support for Within blocks.