Paramaterized gate definition segfault fixed

[moar55]

This pull request fixes #220.

[1tnguyen]

Hi @moar55,

Could you please add a code comment indicating that the gate parameters are of F64 type hence this check (consistent with the earlier code constructing the function signature)? BTW, is there a reason you put the if check after the last_user = arg assignment?

Also, for a bug fix like this, we'd want to add a unit test as well (e.g., just use your example in the bug report as the test case)

[moar55]

No, there is no reason for putting the check after last_user = arg I will push it to the top and add the tests and comments...

[moar55]

Hi @tnguyen-ornl , I have done the changes. Regarding the test, I added test_quantum_declaration.cpp, which has the previously provided gate definition, into the tests cmake file. Some of the tests were problematic though, so I commented them as it is irrelevant to this pull request.

[1tnguyen]

Hi @moar55,

Would you mind cleaning up the git diff? It should be showing up as a single test case added to test_quantum_declaration.cpp.

[moar55]

Hello @tnguyen-ornl, The git diff is because the checkSubroutine tests have issues and cause the tests to fail, so I commented them, and fixing them is beyond the scope of this pull request. Moreover the execute function in checkSecondIfStmt cannot be executed along with that of checkGate, it throws a "_cannot create std::vector larger than maxsize()" error. However, each of these test cases succeed when ran individually, leading me to beleive that that a tear-down is not established between test cases - an issue also out of the scope of this pull request. I can just unadd test_quantum_declaration.cpp from the cmake file, create a new test file, and add this test case, to avoid making the git diff messy. What do you think?

[1tnguyen]

I don't fully understand the issue here. As of the current master branch, all the tests are passing.

Do you mean that adding the conditional block

if (arg.getType().isF64()) {
         result_qubit_vals.push_back(arg);
     // skip use chain traversal
         continue;
}

causes some existing tests to fail? Did you run ctest in parallel (-j)?

[moar55]

The tests are passing in master because test_quantum_declaration.cpp isn't included in the tests cmake file. If you include it you will notice that the tests indeed faill, without the addition I added.

[1tnguyen]

ah, I see. I think that test file is outdated so you don't need to try adding the new test to that.

To keep the git history clean, could you please add the new test the test_declaration.cpp for the time being? We'll look into porting those old tests in test_quantum_declaration.cpp later. Thanks!

[moar55]

okay done :)

[1tnguyen]

Thanks, @moar55! This looks good to me.