[MLIR] Removes use of ValueRange after using sanitizers.

[erick-xanadu]

Context: Wheels did not pass because of memory issues. After looking into it with address sanitizers, ValueRange is the culprit.

Description of the Change: Do not use ValueRange.

https://github.com/PennyLaneAI/catalyst/actions/runs/6495027163

[github-actions]

Hello. You may have forgotten to update the changelog!
Please edit doc/changelog.md on your branch with:

• A one-to-two sentence description of the change. You may include a small working example for new features.
• A link back to this PR.
• Your name (or GitHub username) in the contributors section.

[maliasadi]

Thanks Erick! 🎉

[erick-xanadu]

I'll leave this to @dime10 to decide how he wants to merge for the release. Merge with release candidate or merge with main and then with release candidate.

[dime10]

Just a note here for future reference. std::vectors should generally not be returned from such utility functions that merely provide access to existing data. ValueRange, TypeRange, and the like are used in MLIR precisely to have a lightweight way of passing around IR objects which are tied to a long-lived object.
In this case, it's the Operation object that owns the data (the operands) and there should be no issue returning a range. The actual problem was the way the range was constructed (my bad)!

On the other hand, a common error I've made in the past is constructing a ValueRange locally to be passed into an MLIR function. This typically doesn't work because there is no underlying storage object when gathering various Value objects into a ValueRange in this manner. Here, constructing a container that can hold the disparate Value objects (such as SmallVector<Value>) is necessary.

[dime10]

What about CustomOp? Is that safe because it already returns a range for getParams()?

[erick-xanadu]

What about CustomOp? Is that safe because it already returns a range for getParams()?

When I run all the lit tests with sanitizers there are no more errors.

[dime10]

What about CustomOp? Is that safe because it already returns a range for getParams()?

When I run all the lit tests with sanitizers there are no more errors.

I guess that must be explanation then 😅 Sounds good 👍

[dime10]

It's very curious that we already used the correct form here:

catalyst/mlir/include/Quantum/IR/QuantumOps.td

Line 256 in 9365c72

return getODSOperands(0);

I can't mark it in the code, but could you update that line as well with getParamOperandIdx?

[dime10]

🦝

[dime10]

Oh and update the existing changelog entry with this PR link :)