Panic in QIR generation: "only some primitive types are supported" #2831
Conversation
| #[error("cannot use a dynamic value of type `{0}` returned from intrinsic callable")] | ||
| #[diagnostic(code("Qsc.PartialEval.UnexpectedDynamicIntrsinicReturnType"))] | ||
| UnexpectedDynamicIntrinsicReturnType(String, #[label] PackageSpan), | ||
| #[error("cannot use a dynamic value of type `{0}` used in intrinsic callable signature")] |
There was a problem hiding this comment.
This error message isn't quite making sense to me, at least for the repro I saw. There are no dynamic values in the repro. Also "value" is a runtime concept while "callable signature" is a type, so the phrasing is a bit confusing.
Maybe more to the point, shouldn't custom intrinsics with an invalid signature just be a compiler error, then? We can't run them. We can't estimate them. We can't generate QIR for them. What are they good for?
There was a problem hiding this comment.
I debated on this, and I'm not sure. We have intrinsic callables with arrays in the signature as part of the stdlib, we just also have runtime/codegen support for them. To make this a compile time check, we'd have to not just check the signature of intrinsic callables but also check the names against a whitelist that we keep up to date with the runtime support. That's certainly doable, just felt weird to me. If that's an acceptable implementation quirk, then this can indeed become a compile time check.
In fact, this is a compile time check that becomes easier with some of the updates I'd like to make to RCA...
There was a problem hiding this comment.
After some discussion, we settled for now on an improved error message for clarity. We can circle back to what it would take to pull this into a design-time check later.
Fixes #2830 Also updates package-lock.json at the root to follow npm 10.x conventions.
Co-authored-by: Mine Starks <[email protected]>
swernli
force-pushed
the
swernli/issue2830
branch
from
c1484ce to
f3a271b
Compare
|
Rebased on main to remove the update of package-lock.json |
Fixes #2830