Skip to content

Enforce type safety in NeighborTable#10774

Merged
mtreinish merged 1 commit into
Qiskit:mainfrom
jakelishman:rust/typesafe-neighbours
Sep 6, 2023
Merged

Enforce type safety in NeighborTable#10774
mtreinish merged 1 commit into
Qiskit:mainfrom
jakelishman:rust/typesafe-neighbours

Conversation

@jakelishman
Copy link
Copy Markdown
Member

Summary

This hides the internal .neighbors field in favour of instead implemented Index<PhysicalQubit> on the entire NeighborTable struct. This is done because indexing into .neighbors required calling PhysicalQubit::index to retrieve a usize, but a similar method also exists on VirtualQubit. This meant that it was previously a normal API pattern to do neighbors.neighbors[qubit.index()], and this threw away the type safety: it would compile without warning whether qubit was physical or virtual.

To support the hiding of the .neighbors field, the constructor of the coupling map is moved to be an associated function on NeighborTable, so the unsafe access only happens within the context of all other unsafe operations.

Details and comments

Follow-on from #10761, which this PR depends on.

@jakelishman jakelishman added Changelog: None Do not include in the GitHub Release changelog. Rust This PR or issue is related to Rust code in the repository mod: transpiler Issues and PRs related to Transpiler labels Sep 5, 2023
@jakelishman jakelishman requested a review from a team as a code owner September 5, 2023 16:51
@qiskit-bot
Copy link
Copy Markdown
Collaborator

One or more of the the following people are requested to review this:

@jakelishman jakelishman added the on hold Can not fix yet label Sep 5, 2023
This hides the internal `.neighbors` field in favour of instead
implemented `Index<PhysicalQubit>` on the entire `NeighborTable` struct.
This is done because indexing into `.neighbors` required calling
`PhysicalQubit::index` to retrieve a `usize`, but a similar method also
exists on `VirtualQubit`.  This meant that it was previously a normal
API pattern to do `neighbors.neighbors[qubit.index()]`, and this threw
away the type safety: it would compile without warning whether `qubit`
was physical or virtual.

To support the hiding of the `.neighbors` field, the constructor of the
coupling map is moved to be an associated function on `NeighborTable`,
so the unsafe access only happens within the context of all other unsafe
operations.
@jakelishman jakelishman force-pushed the rust/typesafe-neighbours branch from 452ad44 to 3a3a52d Compare September 5, 2023 21:26
@jakelishman jakelishman removed the on hold Can not fix yet label Sep 5, 2023
@jakelishman
Copy link
Copy Markdown
Member Author

Now rebased over #10761.

@mtreinish mtreinish enabled auto-merge September 5, 2023 21:34
@mtreinish mtreinish added this pull request to the merge queue Sep 5, 2023
Merged via the queue into Qiskit:main with commit 4a57411 Sep 6, 2023
@jakelishman jakelishman deleted the rust/typesafe-neighbours branch September 6, 2023 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog. mod: transpiler Issues and PRs related to Transpiler Rust This PR or issue is related to Rust code in the repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants