Skip to content

Commit ac2d417

Browse files
authored
Refactor 5 FFI test sites to NonNull / as_ref().expect() for clearer non-null contracts (#318)
* Refactor 5 FFI test sites to NonNull / as_ref().expect() for clearer non-null contracts * Drop paths filter on dependency-review pull_request trigger so the required check always reports
1 parent c4ef9c6 commit ac2d417

3 files changed

Lines changed: 23 additions & 44 deletions

File tree

.github/workflows/dependency-review.yml

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,12 @@ on:
2222
- '.github/dependabot.yml'
2323
- '.github/workflows/dependency-review.yml'
2424
pull_request:
25+
# No `paths:` filter here on purpose. `dependency-review` is a required
26+
# status check on protected branches; gating it by `paths:` blocks merges
27+
# of PRs that don't touch dep files (GitHub waits forever for a check
28+
# that doesn't fire). `actions/dependency-review-action` is cheap and
29+
# idempotent on no-diff PRs.
2530
branches: [ "main", "master", "development", "dev" ]
26-
paths:
27-
- 'Cargo.toml'
28-
- 'Cargo.lock'
29-
- 'scripts/native_bench/bench_pecos/Cargo.toml'
30-
- 'scripts/native_bench/bench_pecos/Cargo.lock'
31-
- 'pyproject.toml'
32-
- 'python/**/pyproject.toml'
33-
- 'uv.lock'
34-
- 'requirements*.txt'
35-
- '**/requirements*.txt'
36-
- 'package.json'
37-
- 'package-lock.json'
38-
- 'pnpm-lock.yaml'
39-
- 'yarn.lock'
40-
- 'bun.lock'
41-
- 'bun.lockb'
42-
- '.github/dependabot.yml'
43-
- '.github/workflows/dependency-review.yml'
4431

4532
permissions:
4633
contents: read

crates/pecos-qis-ffi/src/ffi.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,18 +1567,16 @@ mod tests {
15671567

15681568
#[test]
15691569
fn test_heap_alloc_and_free() {
1570-
let ptr = unsafe { heap_alloc(100) };
1571-
assert!(!ptr.is_null());
1570+
let ptr = std::ptr::NonNull::new(unsafe { heap_alloc(100) })
1571+
.expect("heap_alloc(100) should return non-null");
15721572

1573-
// SAFETY: `ptr` was just allocated by `heap_alloc(100)` and asserted non-null
1574-
// above; the test scope owns it exclusively until `heap_free` below.
1573+
// SAFETY: `ptr` owns this allocation exclusively for the test scope;
1574+
// `heap_alloc` returns a properly-aligned `u8` allocation.
15751575
unsafe {
1576-
std::ptr::write(ptr, 42u8);
1577-
assert_eq!(std::ptr::read(ptr), 42u8);
1576+
std::ptr::write(ptr.as_ptr(), 42u8);
1577+
assert_eq!(std::ptr::read(ptr.as_ptr()), 42u8);
1578+
heap_free(ptr.as_ptr());
15781579
}
1579-
1580-
// Free should not crash
1581-
unsafe { heap_free(ptr) };
15821580
}
15831581

15841582
#[test]

crates/pecos-qis-ffi/src/lib.rs

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,11 +1046,11 @@ mod tests {
10461046

10471047
// Get pending operations
10481048
let ptr = pecos_get_pending_operations();
1049-
assert!(!ptr.is_null());
1050-
1051-
// SAFETY: `pecos_get_pending_operations` returns a fresh leaked Box;
1052-
// non-null asserted above; the test scope owns it until `pecos_free_operations`.
1053-
let collector = unsafe { &*ptr };
1049+
// SAFETY: `pecos_get_pending_operations` returns null or a leaked
1050+
// `Box<OperationCollector>` (properly initialised + aligned);
1051+
// ownership stays here until `pecos_free_operations` below.
1052+
let collector =
1053+
unsafe { ptr.as_ref() }.expect("pecos_get_pending_operations returned null");
10541054
assert_eq!(collector.operations.len(), 2);
10551055

10561056
// Free the collector
@@ -1341,10 +1341,8 @@ mod tests {
13411341

13421342
// Verify operations were exported
13431343
let ops_ptr = pecos_get_pending_operations();
1344-
assert!(!ops_ptr.is_null());
1345-
// SAFETY: `pecos_get_pending_operations` returns a fresh leaked Box;
1346-
// non-null asserted above; freed below via `pecos_free_operations`.
1347-
let ops = unsafe { &*ops_ptr };
1344+
// SAFETY: see `pecos_get_pending_operations` -- null-or-leaked-Box invariant.
1345+
let ops = unsafe { ops_ptr.as_ref() }.expect("pecos_get_pending_operations returned null");
13481346
assert_eq!(ops.operations.len(), 2);
13491347
unsafe { pecos_free_operations(ops_ptr) };
13501348

@@ -1409,21 +1407,17 @@ mod tests {
14091407
let needed_id = pecos_wait_for_need_result(500);
14101408
assert_eq!(needed_id, 0);
14111409
let ops_ptr = pecos_get_pending_operations();
1412-
assert!(!ops_ptr.is_null());
1413-
// SAFETY: `pecos_get_pending_operations` returns a fresh leaked Box;
1414-
// non-null asserted above; freed below via `pecos_free_operations`.
1415-
let ops = unsafe { &*ops_ptr };
1410+
// SAFETY: see `pecos_get_pending_operations` -- null-or-leaked-Box invariant.
1411+
let ops = unsafe { ops_ptr.as_ref() }.expect("pecos_get_pending_operations returned null");
14161412
assert_eq!(ops.operations, vec![Operation::AllocateQubit { id: 0 }]);
14171413
unsafe { pecos_free_operations(ops_ptr) };
14181414
pecos_signal_result_ready();
14191415

14201416
let needed_id = pecos_wait_for_need_result(500);
14211417
assert_eq!(needed_id, 1);
14221418
let ops_ptr = pecos_get_pending_operations();
1423-
assert!(!ops_ptr.is_null());
1424-
// SAFETY: `pecos_get_pending_operations` returns a fresh leaked Box;
1425-
// non-null asserted above; freed below via `pecos_free_operations`.
1426-
let ops = unsafe { &*ops_ptr };
1419+
// SAFETY: see `pecos_get_pending_operations` -- null-or-leaked-Box invariant.
1420+
let ops = unsafe { ops_ptr.as_ref() }.expect("pecos_get_pending_operations returned null");
14271421
assert_eq!(ops.operations, vec![Operation::Quantum(QuantumOp::H(0))]);
14281422
unsafe { pecos_free_operations(ops_ptr) };
14291423
pecos_signal_result_ready();

0 commit comments

Comments
 (0)