✨ Refactor QDMI Devices and Fix Singleton Handling

[ystade]

Description

This PR contributes to #1331 (but does not fully resolve it).

Checklist:

• The pull request only contains commits that are focused and relevant to this change.
• I have added appropriate tests that cover the new/changed functionality.
• I have updated the documentation to reflect these changes.
• I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.
• I have added migration instructions to the upgrade guide (if needed).
• The changes follow the project's style guidelines and introduce no new warnings.
• The changes are fully tested and pass the CI checks.
• I have reviewed my own code changes.

[codecov]

Codecov Report

❌ Patch coverage is 67.39130% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/qdmi/devices/dd/Device.cpp	58.3%	5 Missing ⚠️
src/qdmi/devices/na/Device.cpp	37.5%	5 Missing ⚠️
src/qdmi/devices/sc/Device.cpp	37.5%	5 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-tidy (v21.1.8) reports: 13 concern(s)

• src/qdmi/devices/dd/Device.cpp:360:23: warning: [misc-include-cleaner]

  no header providing "std::runtime_error" is directly included

     43 |   } catch (const std::runtime_error&) {
        |                       ^

• src/qdmi/devices/dd/Device.cpp:826:5: warning: [bugprone-empty-catch]

  empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether

    826 |   } catch (const std::runtime_error&) {
        |     ^

• src/qdmi/devices/na/App.cpp:403:20: warning: [clang-diagnostic-sign-conversion]

  implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long')

    403 |     argVec.reserve(argc);
        |            ~~~~~~~ ^~~~

• src/qdmi/devices/na/App.cpp:404:44: warning: [clang-diagnostic-sign-conversion]

  implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long')

    404 |     for (const auto& arg : std::span(argv, argc)) {
        |                            ~~~             ^~~~

• src/qdmi/devices/na/Device.cpp:21:1: warning: [misc-include-cleaner]

  included header atomic is not used directly

     21 | #include <atomic>
        | ^~~~~~~~~~~~~~~~~
     22 | #include <cassert>

• src/qdmi/devices/na/Device.cpp:228:23: warning: [misc-include-cleaner]

  no header providing "std::runtime_error" is directly included

     29 |   } catch (const std::runtime_error&) {
        |                       ^

• src/qdmi/devices/na/Device.cpp:662:5: warning: [readability-avoid-return-with-void-value]

  return statement within a void function should not have a specified return value

    662 |     return qdmi::na::Device::get()->sessionFree(session);
        |     ^~~~~~                                               
        |                                                           return;

• src/qdmi/devices/na/Device.cpp:663:5: warning: [bugprone-empty-catch]

  empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether

    663 |   } catch (const std::runtime_error&) {
        |     ^

• src/qdmi/devices/sc/App.cpp:457:20: warning: [clang-diagnostic-sign-conversion]

  implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long')

    457 |     argVec.reserve(argc);
        |            ~~~~~~~ ^~~~

• src/qdmi/devices/sc/App.cpp:458:44: warning: [clang-diagnostic-sign-conversion]

  implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long')

    458 |     for (const auto& arg : std::span(argv, argc)) {
        |                            ~~~             ^~~~

• src/qdmi/devices/sc/Device.cpp:21:1: warning: [misc-include-cleaner]

  included header atomic is not used directly

     21 | #include <atomic>
        | ^~~~~~~~~~~~~~~~~
     22 | #include <cassert>

• src/qdmi/devices/sc/Device.cpp:222:23: warning: [misc-include-cleaner]

  no header providing "std::runtime_error" is directly included

     29 |   } catch (const std::runtime_error&) {
        |                       ^

• src/qdmi/devices/sc/Device.cpp:495:5: warning: [bugprone-empty-catch]

  empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether

    495 |   } catch (const std::runtime_error&) {
        |     ^

Have any feedback or feature suggestions? Share it here.

[ystade]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced centralized singleton management for device instances with built-in thread-safety.

• Bug Fixes

  • Enhanced error handling for uninitialized device states with improved error reporting.

• Tests

  • Expanded test coverage for device lifecycle management, session handling, and property queries.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

This PR extracts singleton handling from individual QDMI device implementations into a reusable generic SingletonDevice<T> base template class, reorganizes device source files from scattered locations into a unified qdmi/devices/ directory structure, and updates all dependent code to use the new singleton API (returning shared_ptr<Device> instead of direct references), with added exception-based error handling for uninitialized state.

Changes

Cohort / File(s)	Summary
Documentation CHANGELOG.md	Added changelog entry for PR #1347 describing extraction and thread-safe refactoring of QDMI device singleton handling.
New Singleton Base Template include/mqt-core/qdmi/devices/base/Device.hpp	Introduced generic SingletonDevice<ConcreteType> class template with protected constructor, virtual destructor, deleted copy/move semantics, and public static lifecycle methods: initialize(), finalize(), and get() returning shared_ptr<ConcreteType>. Includes mutex for thread-safety and friendship with std::shared_ptr<ConcreteType>.
Device Header Refactoring include/mqt-core/qdmi/devices/dd/Device.hpp, include/mqt-core/qdmi/devices/na/Device.hpp, include/mqt-core/qdmi/devices/sc/Device.hpp	Updated Device classes to inherit from SingletonDevice<Device>, removed static singleton instance and lifecycle methods (initialize(), finalize(), get()), changed destructor to override base virtual destructor, deleted move operations, and updated include paths.
Include Path Updates bindings/na/fomac/fomac.cpp, include/mqt-core/na/fomac/Device.hpp, src/na/fomac/Device.cpp, src/qdmi/devices/na/App.cpp, src/qdmi/devices/na/Generator.cpp, src/qdmi/devices/sc/App.cpp, src/qdmi/devices/sc/Generator.cpp, test/fomac/test_fomac.cpp, test/qdmi/devices/na/test_*.cpp	Updated Generator header include paths from na/device/Generator.hpp and qdmi/sc/Generator.hpp to new locations under qdmi/devices/.
Device Implementation Refactoring src/qdmi/devices/dd/Device.cpp, src/qdmi/devices/na/Device.cpp, src/qdmi/devices/sc/Device.cpp	Removed static singleton instance and lifecycle management; updated all call sites from Device::get().method() to Device::get()->method() to use shared_ptr accessor; added try-catch blocks around get() calls to map std::runtime_error to QDMI_ERROR_BADSTATE in device operations (queryDeviceProperty, session management, job submission).
CMake Restructuring src/qdmi/CMakeLists.txt, src/qdmi/devices/CMakeLists.txt, src/qdmi/devices/base/CMakeLists.txt, src/qdmi/devices/dd/CMakeLists.txt, src/qdmi/devices/na/CMakeLists.txt, src/qdmi/devices/sc/CMakeLists.txt, src/qdmi/driver/CMakeLists.txt	Reorganized QDMI build structure: introduced devices/ subdirectory with separate base, na, dd, sc device targets; created new MQT::CoreQDMIBaseDevice INTERFACE library; moved driver build configuration to separate driver/ subdirectory; updated header paths and target dependencies.
Build Configuration Updates src/na/CMakeLists.txt, test/na/CMakeLists.txt	Removed device subdirectory from NA build; cleaned up test structure.
Test Infrastructure test/qdmi/CMakeLists.txt, test/qdmi/devices/CMakeLists.txt, test/qdmi/devices/sc/CMakeLists.txt, test/qdmi/driver/CMakeLists.txt, test/qdmi/driver/test_driver.cpp	Restructured test configuration to parallel new device directory layout; added SC device test suite with conditional setup for targets; introduced driver test target with platform-specific dynamic library dependencies and compile definitions.
New Test Files test/qdmi/devices/sc/test_*.cpp	Added comprehensive SC device tests: test_app.cpp with executable integration tests (version, help, schema generation), test_device.cpp with QDMI interface lifecycle and property tests, test_generator.cpp with JSON schema validation.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client as Caller Code
    participant Lifecycle as Lifecycle Manager
    participant SingletonDevice as SingletonDevice<T>
    participant Device as Device Implementation
    participant SharedPtr as shared_ptr<Device>
    participant Mutex as Thread Mutex

    Client->>Lifecycle: initialize()
    Lifecycle->>Mutex: acquire lock
    alt not initialized
        Mutex->>Device: construct via make_shared()
        Device->>SharedPtr: new shared_ptr
        SharedPtr->>SingletonDevice: store instance pointer
    end
    Mutex->>Lifecycle: release lock

    Client->>Client: call get() later
    Client->>SingletonDevice: get()
    SingletonDevice->>Mutex: acquire lock
    alt instance exists
        SingletonDevice->>SharedPtr: return shared_ptr copy
        SharedPtr->>Client: provide shared_ptr<Device>
    else not initialized
        SingletonDevice->>Client: throw runtime_error
    end
    Mutex->>SingletonDevice: release lock

    Client->>SharedPtr: use -> accessor
    SharedPtr->>Device: delegate call

    Client->>Lifecycle: finalize()
    Lifecycle->>Mutex: acquire lock (then leak)
    Mutex->>SingletonDevice: reset instance pointer
    Lifecycle->>Client: complete (SharedPtr refs remain valid)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

• Areas requiring extra attention:
  • Thread-safety guarantees in SingletonDevice<T> template: validate mutex ownership, shared_ptr copy semantics, and leak-by-design of mutex in finalize() to prevent static deinitialization races
  • Consistency of Device::get()->method() conversion across all three device implementations (DD, NA, SC); verify error handling covers all initialization paths and maps exceptions correctly to QDMI_ERROR_BADSTATE
  • CMake target dependency chain: ensure MQT::CoreQDMIBaseDevice INTERFACE library is correctly linked by all device targets and that parent scope propagation of MQT_CORE_TARGETS maintains correct visibility
  • Test setup and dynamic library loading in driver tests: validate compile definitions and platform-specific paths are correctly resolved at build and runtime

Possibly related PRs

• 🐛 Fix dynamic QDMI Device #1336: Modifies Device singleton lifecycle for QDMI devices, introducing early similar changes to initialize()/finalize()/get() patterns before the unified SingletonDevice base was extracted.
• ✨ New QDMI Device to represent SC Devices #1328: Refactors QDMI device layout and headers (moving SC device to qdmi/devices/sc), which this PR builds upon to unify all devices under the new directory structure and add singleton inheritance.
• ✨ DD-based Simulator QDMI device #1287: Introduces DD device implementation and its singleton API, which this PR refactors to inherit from the new generic SingletonDevice base template.

Suggested reviewers

• burgholzer

---

🐰 A singleton hops into the fold,
Thread-safe and shared, no more alone!
Base class wisdom, reused and bold,
Devices dance in devices zone!
Refactored paths, one home to hold. 🏠✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description references issue #1331 but lacks detailed explanation of changes. Most checklist items are unchecked, including documentation and changelog updates despite CHANGELOG.md modifications present.	Add detailed summary of the refactoring changes, mark completed checklist items (documentation, changelog were updated), and clarify the relationship to issue #1331.
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.09% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: refactoring QDMI devices and fixing singleton handling through the introduction of a generic singleton base template.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ystade/qdmi-sc-device-polish

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[burgholzer]

The conflicts here stem from the merge of #1328.
I hope they are quick enough to resolve.

[coderabbitai]

Actionable comments posted: 9

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02fda43 and 40405cb.

📒 Files selected for processing (36)

• CHANGELOG.md (2 hunks)
• bindings/na/fomac/fomac.cpp (1 hunks)
• include/mqt-core/na/fomac/Device.hpp (1 hunks)
• include/mqt-core/qdmi/devices/base/Device.hpp (1 hunks)
• include/mqt-core/qdmi/devices/dd/Device.hpp (4 hunks)
• include/mqt-core/qdmi/devices/na/Device.hpp (3 hunks)
• include/mqt-core/qdmi/devices/sc/Device.hpp (2 hunks)
• src/na/CMakeLists.txt (0 hunks)
• src/na/fomac/Device.cpp (1 hunks)
• src/qdmi/CMakeLists.txt (1 hunks)
• src/qdmi/devices/CMakeLists.txt (1 hunks)
• src/qdmi/devices/base/CMakeLists.txt (1 hunks)
• src/qdmi/devices/dd/CMakeLists.txt (1 hunks)
• src/qdmi/devices/dd/Device.cpp (7 hunks)
• src/qdmi/devices/na/App.cpp (1 hunks)
• src/qdmi/devices/na/CMakeLists.txt (5 hunks)
• src/qdmi/devices/na/Device.cpp (4 hunks)
• src/qdmi/devices/na/Generator.cpp (1 hunks)
• src/qdmi/devices/sc/App.cpp (1 hunks)
• src/qdmi/devices/sc/CMakeLists.txt (5 hunks)
• src/qdmi/devices/sc/Device.cpp (4 hunks)
• src/qdmi/devices/sc/Generator.cpp (2 hunks)
• src/qdmi/driver/CMakeLists.txt (1 hunks)
• src/qdmi/driver/Driver.cpp (1 hunks)
• test/fomac/test_fomac.cpp (1 hunks)
• test/na/CMakeLists.txt (0 hunks)
• test/qdmi/CMakeLists.txt (1 hunks)
• test/qdmi/devices/CMakeLists.txt (1 hunks)
• test/qdmi/devices/na/test_device.cpp (1 hunks)
• test/qdmi/devices/na/test_generator.cpp (1 hunks)
• test/qdmi/devices/sc/CMakeLists.txt (1 hunks)
• test/qdmi/devices/sc/test_app.cpp (1 hunks)
• test/qdmi/devices/sc/test_device.cpp (1 hunks)
• test/qdmi/devices/sc/test_generator.cpp (1 hunks)
• test/qdmi/driver/CMakeLists.txt (1 hunks)
• test/qdmi/driver/test_driver.cpp (1 hunks)

💤 Files with no reviewable changes (2)

• test/na/CMakeLists.txt
• src/na/CMakeLists.txt

🧰 Additional context used

🧠 Learnings (11)

📓 Common learnings

Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.

Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/test_qdmi_qiskit_site_handling.py:162-173
Timestamp: 2025-10-17T11:09:50.147Z
Learning: For QDMI tests that reference a specific named device via fixture (e.g., "MQT NA Default QDMI Device"), it is acceptable to hard-code device-specific values such as zone counts and indices, as these tests are intentionally coupled to that device's structure and serve as structural invariant checks.

📚 Learning: 2025-11-03T23:09:26.881Z

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.

Applied to files:

• test/qdmi/devices/CMakeLists.txt
• include/mqt-core/na/fomac/Device.hpp
• test/qdmi/devices/na/test_generator.cpp
• test/qdmi/CMakeLists.txt
• src/qdmi/devices/base/CMakeLists.txt
• test/qdmi/devices/sc/CMakeLists.txt
• src/qdmi/driver/CMakeLists.txt
• src/qdmi/CMakeLists.txt
• bindings/na/fomac/fomac.cpp
• test/qdmi/driver/CMakeLists.txt
• test/qdmi/devices/na/test_device.cpp
• test/qdmi/driver/test_driver.cpp
• src/qdmi/driver/Driver.cpp
• src/qdmi/devices/CMakeLists.txt
• src/qdmi/devices/sc/App.cpp
• src/qdmi/devices/na/App.cpp
• include/mqt-core/qdmi/devices/dd/Device.hpp
• test/qdmi/devices/sc/test_device.cpp
• src/qdmi/devices/na/Generator.cpp
• src/na/fomac/Device.cpp
• src/qdmi/devices/sc/Generator.cpp
• src/qdmi/devices/sc/CMakeLists.txt
• src/qdmi/devices/dd/Device.cpp
• src/qdmi/devices/dd/CMakeLists.txt
• src/qdmi/devices/na/CMakeLists.txt

📚 Learning: 2025-10-17T11:09:50.147Z

Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/test_qdmi_qiskit_site_handling.py:162-173
Timestamp: 2025-10-17T11:09:50.147Z
Learning: For QDMI tests that reference a specific named device via fixture (e.g., "MQT NA Default QDMI Device"), it is acceptable to hard-code device-specific values such as zone counts and indices, as these tests are intentionally coupled to that device's structure and serve as structural invariant checks.

Applied to files:

• test/qdmi/devices/CMakeLists.txt
• test/qdmi/devices/na/test_generator.cpp
• test/qdmi/CMakeLists.txt
• src/qdmi/devices/base/CMakeLists.txt
• test/qdmi/devices/sc/CMakeLists.txt
• test/qdmi/driver/CMakeLists.txt
• test/qdmi/devices/na/test_device.cpp
• src/qdmi/driver/Driver.cpp
• include/mqt-core/qdmi/devices/sc/Device.hpp
• include/mqt-core/qdmi/devices/base/Device.hpp
• src/qdmi/devices/sc/Device.cpp
• include/mqt-core/qdmi/devices/dd/Device.hpp
• test/qdmi/devices/sc/test_device.cpp
• src/qdmi/devices/na/Generator.cpp
• src/qdmi/devices/dd/Device.cpp
• src/qdmi/devices/na/Device.cpp
• include/mqt-core/qdmi/devices/na/Device.hpp
• src/qdmi/devices/dd/CMakeLists.txt
• src/qdmi/devices/na/CMakeLists.txt

📚 Learning: 2025-11-04T14:28:32.371Z

Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/test_qdmi_qiskit_backend.py:0-0
Timestamp: 2025-11-04T14:28:32.371Z
Learning: In the munich-quantum-toolkit/core repository, at least one FoMaC device is always available during testing, so skip logic for missing devices in QDMI Qiskit backend tests is not necessary.

Applied to files:

• test/qdmi/CMakeLists.txt
• test/qdmi/devices/sc/CMakeLists.txt
• bindings/na/fomac/fomac.cpp
• test/qdmi/driver/CMakeLists.txt
• include/mqt-core/qdmi/devices/base/Device.hpp
• src/qdmi/devices/sc/Device.cpp
• test/qdmi/devices/sc/test_device.cpp
• src/qdmi/devices/dd/Device.cpp
• src/qdmi/devices/na/Device.cpp

📚 Learning: 2025-11-05T07:42:45.507Z

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/error_handling_test.cpp:118-194
Timestamp: 2025-11-05T07:42:45.507Z
Learning: In the munich-quantum-toolkit/core QDMI device API, session parameters can only be set before calling `device_session_init()`. Once a session is initialized, any attempt to set a parameter returns `QDMI_ERROR_BADSTATE`. Since `SessionGuard` (in test/qdmi/dd/helpers/test_utils.hpp) automatically initializes the session in its constructor, tests that need to verify session parameter setting behavior before initialization must allocate a separate uninitialized session rather than reusing the `SessionGuard`'s session.

Applied to files:

• include/mqt-core/qdmi/devices/sc/Device.hpp
• include/mqt-core/qdmi/devices/base/Device.hpp
• src/qdmi/devices/sc/Device.cpp
• include/mqt-core/qdmi/devices/dd/Device.hpp
• test/qdmi/devices/sc/test_device.cpp
• src/qdmi/devices/dd/Device.cpp
• src/qdmi/devices/na/Device.cpp
• include/mqt-core/qdmi/devices/na/Device.hpp

📚 Learning: 2025-10-14T14:37:38.047Z

Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.

Applied to files:

• CHANGELOG.md

📚 Learning: 2025-12-01T11:00:40.342Z

Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: CHANGELOG.md:18-18
Timestamp: 2025-12-01T11:00:40.342Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the CHANGELOG.md intentionally references the parent MQT Core repository's release notes (https://github.com/munich-quantum-toolkit/core/releases) because the plugin repository is based on work performed in the parent repository.

Applied to files:

• CHANGELOG.md

📚 Learning: 2025-10-10T08:09:54.528Z

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.

Applied to files:

• src/qdmi/devices/CMakeLists.txt
• src/qdmi/devices/sc/CMakeLists.txt

📚 Learning: 2025-10-10T08:10:16.394Z

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: test/python/na/test_na_fomac.py:35-0
Timestamp: 2025-10-10T08:10:16.394Z
Learning: In the munich-quantum-toolkit/core repository, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which means CMake `install()` commands with `DESTINATION <path>` install files relative to `mqt/core/` in the wheel, making them accessible via `files("mqt.core").joinpath("<path>")`.

Applied to files:

• src/qdmi/devices/CMakeLists.txt

📚 Learning: 2025-11-04T19:50:14.489Z

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/helpers/test_utils.cpp:103-118
Timestamp: 2025-11-04T19:50:14.489Z
Learning: In the munich-quantum-toolkit/core QDMI device tests, when QDMI APIs return a string result size `ks` that includes a null terminator, the pattern `std::string keys(ks - 1, '\0'); ... device_job_get_results(..., ks, keys.data(), nullptr)` is correct and safe. The string allocates `ks` bytes total (ks-1 content + implicit null), and the API writing `ks` bytes (including trailing null) fits exactly into this space without buffer overrun.

Applied to files:

• test/qdmi/devices/sc/test_device.cpp

📚 Learning: 2025-10-09T13:14:10.178Z

Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:219-221
Timestamp: 2025-10-09T13:14:10.178Z
Learning: The MQT Core project (munich-quantum-toolkit/core repository) uses the C++20 standard, not C++17. C++20 features such as abbreviated function templates (e.g., `const auto&` parameters) are supported and valid in this codebase.

Applied to files:

• src/qdmi/devices/sc/CMakeLists.txt
• src/qdmi/devices/na/CMakeLists.txt

🧬 Code graph analysis (5)

include/mqt-core/qdmi/devices/sc/Device.hpp (2)

include/mqt-core/qdmi/devices/dd/Device.hpp (4)

• Device (66-66)
• Device (70-70)
• Device (73-73)
• Device (77-77)

include/mqt-core/qdmi/devices/na/Device.hpp (4)

• Device (71-71)
• Device (75-75)
• Device (78-78)
• Device (82-82)

include/mqt-core/qdmi/devices/dd/Device.hpp (1)

src/qdmi/devices/dd/Device.cpp (1)

• Device (212-214)

test/qdmi/devices/sc/test_device.cpp (1)

src/qdmi/devices/sc/Device.cpp (33)

• MQT_SC_QDMI_device_session_query_device_property (577-584)
• MQT_SC_QDMI_device_session_query_device_property (577-579)
• MQT_SC_QDMI_device_session_alloc (474-482)
• MQT_SC_QDMI_device_session_alloc (474-474)
• MQT_SC_QDMI_device_session_init (484-489)
• MQT_SC_QDMI_device_session_init (484-484)
• MQT_SC_QDMI_device_session_free (491-497)
• MQT_SC_QDMI_device_session_free (491-491)
• MQT_SC_QDMI_device_finalize (469-472)
• MQT_SC_QDMI_device_finalize (469-469)
• MQT_SC_QDMI_device_session_create_device_job (508-514)
• MQT_SC_QDMI_device_session_create_device_job (508-509)
• MQT_SC_QDMI_device_job_free (516-516)
• MQT_SC_QDMI_device_job_free (516-516)
• MQT_SC_QDMI_device_session_set_parameter (499-506)
• MQT_SC_QDMI_device_session_set_parameter (499-501)
• MQT_SC_QDMI_device_job_set_parameter (518-525)
• MQT_SC_QDMI_device_job_set_parameter (518-520)
• MQT_SC_QDMI_device_job_query_property (527-535)
• MQT_SC_QDMI_device_job_query_property (527-530)
• MQT_SC_QDMI_device_job_submit (537-542)
• MQT_SC_QDMI_device_job_submit (537-537)
• MQT_SC_QDMI_device_job_cancel (544-549)
• MQT_SC_QDMI_device_job_cancel (544-544)
• MQT_SC_QDMI_device_job_check (551-557)
• MQT_SC_QDMI_device_job_check (551-552)
• MQT_SC_QDMI_device_job_wait (559-565)
• MQT_SC_QDMI_device_job_wait (559-560)
• MQT_SC_QDMI_device_job_get_results (567-575)
• MQT_SC_QDMI_device_job_get_results (567-570)
• site (307-307)
• MQT_SC_QDMI_device_session_query_site_property (586-594)
• MQT_SC_QDMI_device_session_query_site_property (586-589)

test/qdmi/devices/sc/test_generator.cpp (1)

src/qdmi/devices/sc/Generator.cpp (4)

• writeJSONSchema (150-163)
• writeJSONSchema (150-150)
• writeJSONSchema (175-186)
• writeJSONSchema (175-175)

include/mqt-core/qdmi/devices/na/Device.hpp (1)

include/mqt-core/qdmi/devices/dd/Device.hpp (4)

• Device (66-66)
• Device (70-70)
• Device (73-73)
• Device (77-77)

🪛 GitHub Check: 🇨‌ Lint / 🚨 Lint

src/qdmi/devices/dd/Device.cpp

[warning] 360-360: src/qdmi/devices/dd/Device.cpp:360:23 [misc-include-cleaner]
no header providing "std::runtime_error" is directly included

---

[warning] 826-826: src/qdmi/devices/dd/Device.cpp:826:5 [bugprone-empty-catch]
empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether

src/qdmi/devices/na/Device.cpp

[warning] 228-228: src/qdmi/devices/na/Device.cpp:228:23 [misc-include-cleaner]
no header providing "std::runtime_error" is directly included

---

[warning] 663-663: src/qdmi/devices/na/Device.cpp:663:5 [bugprone-empty-catch]
empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether

---

[warning] 662-662: src/qdmi/devices/na/Device.cpp:662:5 [readability-avoid-return-with-void-value]
return statement within a void function should not have a specified return value

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
• GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
• GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022

🔇 Additional comments (55)

bindings/na/fomac/fomac.cpp (1)

15-15: Include path update to new QDMI device location looks correct

Switching to #include "qdmi/devices/na/Generator.hpp" is consistent with the new unified QDMI device layout and does not alter the binding logic in this file.

src/na/fomac/Device.cpp (1)

15-15: Include path realignment looks correct

The switch to qdmi/devices/na/Generator.hpp matches the new device header layout and keeps this TU consistent with the rest of the refactor.

src/qdmi/devices/dd/CMakeLists.txt (1)

37-50: DD device now correctly depends on base QDMI device interface

Using qdmi/devices/dd/Device.hpp in the FILE_SET and adding MQT::CoreQDMIBaseDevice as a private dependency cleanly tie the DD device into the new shared base-device infrastructure. The wiring here looks consistent with the refactor.

test/qdmi/devices/na/test_generator.cpp (1)

11-11: Test include updated to new generator location

The test now includes qdmi/devices/na/Generator.hpp, which matches the refactored header layout used by the NA device implementation and CLI. Looks good.

src/qdmi/devices/na/App.cpp (1)

11-11: CLI now includes generator from unified qdmi/devices/na path

The include adjustment keeps the NA generator CLI aligned with the new header layout; no behavioral impact.

src/qdmi/driver/Driver.cpp (1)

11-11: Driver include path matches new header layout

Switching to "qdmi/driver/Driver.hpp" is consistent with the reorganized QDMI driver module. No functional changes here.

src/qdmi/devices/na/Generator.cpp (1)

15-15: NA generator implementation now uses the consolidated header path

Including qdmi/devices/na/Generator.hpp here keeps the implementation consistent with the new device/generator directory structure; no behavioral issues.

test/fomac/test_fomac.cpp (1)

19-19: Explicitly including <optional> matches the std::optional usage in the operation tests.

This removes reliance on transitive includes and is consistent with the rest of the file.

CHANGELOG.md (1)

24-24: New changelog entry and PR link follow existing style and guidelines.

The entry for extracting thread‑safe singleton handling and its [#1347] reference are consistent with the surrounding entries (emoji, title, PR ref, contributor handle) and do not introduce issue references, which matches the documented changelog conventions.

As per coding guidelines / learnings, this format looks correct.

Also applies to: 275-275

test/qdmi/driver/test_driver.cpp (1)

11-11: Driver header path update aligns with the new driver layout.

Including "qdmi/driver/Driver.hpp" is consistent with the refactored QDMI driver structure and should continue to expose qdmi::Driver and DYN_DEV_LIBS as used below.

test/qdmi/devices/na/test_device.cpp (1)

12-12: NA generator include path correctly reflects the new device header layout.

Switching to "qdmi/devices/na/Generator.hpp" matches the refactored NA device/generator structure while preserving the expected JSON‑backed invariants these tests assert on.

Based on learnings about NA QDMI tests being intentionally device‑specific, this change keeps that contract intact.

src/qdmi/devices/sc/App.cpp (1)

11-11: SC generator include path matches the new qdmi/devices/sc structure.

The updated include keeps sc::Device and the generator helpers in sync with the reorganized device directory without affecting the CLI logic.

src/qdmi/devices/sc/Generator.cpp (1)

15-18: Generator header relocation and doc tweak are consistent and non‑breaking.

Including "qdmi/devices/sc/Generator.hpp" matches the new device layout; the added <cassert> and @returns tag do not alter behavior and keep the implementation ready for future assertions and clearer documentation.

Also applies to: 218-218

test/qdmi/devices/CMakeLists.txt (1)

1-11: New device test CMake aggregator is straightforward and appropriate.

Adding na, dd, and sc subdirectories centralizes QDMI device tests cleanly, leaving per‑device CMake files to manage their own conditions and targets.

include/mqt-core/na/fomac/Device.hpp (1)

14-14: FoMaC NA device now correctly includes the relocated NA generator header.

Referencing "qdmi/devices/na/Generator.hpp" keeps na::FoMaC::Device wired to the same na::Device definition after the header move, without altering its public interface.

src/qdmi/devices/CMakeLists.txt (1)

1-17: LGTM!

The CMake structure cleanly organizes device subdirectories and correctly propagates the target list to parent scope.

test/qdmi/devices/sc/test_generator.cpp (2)

23-39: LGTM!

The recursive validation helper correctly ensures arrays are populated with non-null entries and properly validates nested structures.

---

42-52: LGTM!

The test case comprehensively validates JSON schema generation by checking both successful parsing and structural integrity through the recursive validation helper.

test/qdmi/CMakeLists.txt (1)

9-10: LGTM!

The simplified structure cleanly delegates test wiring to the respective subdirectories, improving maintainability and aligning with the device reorganization.

test/qdmi/devices/sc/test_app.cpp (4)

18-28: LGTM!

Platform abstraction for popen/pclose is correctly implemented with appropriate conditional compilation for Windows and POSIX systems.

---

30-144: LGTM!

The test cases properly validate command-line interface behavior including version output format, error handling for missing/unknown commands, and argument validation. The consistent test pattern enhances readability and maintainability.

---

279-371: LGTM!

The round-trip tests provide excellent end-to-end validation by ensuring generated schemas are valid. The file-based variant properly cleans up after itself to maintain test isolation.

---

183-277: LGTM!

The usage/help tests comprehensively validate that help output is available and contains expected descriptions for all subcommands, ensuring good user experience.

src/qdmi/CMakeLists.txt (1)

9-15: LGTM!

The reorganization cleanly delegates device and driver target creation to their respective subdirectories, improving modularity and maintainability.

test/qdmi/devices/sc/test_device.cpp (6)

23-41: LGTM!

The querySites helper follows the standard QDMI two-phase query pattern and properly handles errors with descriptive exceptions.

---

44-89: LGTM!

The test fixture hierarchy is well-designed with proper RAII cleanup, ensuring resources are freed in the correct order. The helpful error message in SetUp aids debugging authentication or connectivity issues.

---

91-119: LGTM!

The session lifecycle tests properly validate error handling for null arguments, bad states, and parameter setting timing. The use of testing::AnyOf correctly handles optional feature support.

---

121-240: LGTM!

The job lifecycle tests comprehensively validate creation, parameter setting, property queries, and operations (submit, cancel, check, wait, get results). The proper use of GTEST_SKIP() for unsupported features and testing::AnyOf for optional behavior demonstrates robust test design.

---

242-277: LGTM!

The property query tests thoroughly validate error handling for device and site properties across various invalid argument and state conditions.

---

279-342: LGTM!

The specific property tests validate required QDMI device properties (name, version, library version) and ensure they're non-empty. The string allocation pattern correctly accounts for the null terminator.

Based on learnings, the pattern std::string value(size - 1, '\0') followed by an API call writing size bytes is correct and safe.

include/mqt-core/qdmi/devices/sc/Device.hpp (3)

30-31: LGTM!

The class correctly inherits from SingletonDevice<Device> using CRTP and declares it as a friend to enable the base class to access the private constructor. The final specifier appropriately prevents further derivation.

---

18-18: LGTM!

The include path correctly references the new base device header location under the reorganized directory structure.

---

64-64: LGTM!

The override specifier correctly indicates this destructor overrides the virtual destructor in the SingletonDevice base class, aligning with the singleton refactoring.

src/qdmi/driver/CMakeLists.txt (1)

9-33: LGTM!

The QDMI driver target is correctly configured with:

• Proper target guard to prevent duplicate definition
• Modern FILE_SET HEADERS for header exposure
• Appropriate public/private linkage visibility
• Correct propagation to parent scope via MQT_CORE_TARGETS

The driver appropriately depends on all three device implementations (NA, SC, DD_DDSIM).

test/qdmi/devices/sc/CMakeLists.txt (1)

1-31: LGTM!

The CMake configuration follows the established pattern from other QDMI device tests (na, dd). The conditional target check, QDMI prefix setup, and dependency chain are properly configured. The compile definitions correctly use generator expressions for paths and version info.

src/qdmi/devices/sc/Device.cpp (3)

15-18: LGTM!

Include paths correctly updated to reflect the new qdmi/devices/sc/ directory structure.

---

219-225: LGTM!

Appropriate exception handling for the new singleton pattern. Catching std::runtime_error from get() and translating it to QDMI_ERROR_BADSTATE is the correct approach for API consistency.

---

475-481: LGTM!

Consistent exception handling pattern with queryDeviceProperty. The error translation to QDMI_ERROR_BADSTATE maintains API contract.

src/qdmi/devices/na/CMakeLists.txt (4)

26-28: LGTM!

Header paths correctly updated to the new qdmi/devices/na/ directory structure.

---

96-99: LGTM!

Generated header paths and directory creation properly aligned with the new directory structure.

---

130-137: LGTM!

Device header path updated and the new MQT::CoreQDMIBaseDevice dependency correctly added to support the SingletonDevice<T> base class inheritance.

---

180-180: LGTM!

Dynamic device header path consistent with the static device configuration.

test/qdmi/driver/CMakeLists.txt (1)

1-21: LGTM!

The driver test configuration is well-structured with appropriate platform-specific handling. The DYN_DEV_LIBS compile definition cleverly uses C++17 CTAD (Class Template Argument Deduction) for std::array and std::pair, which is supported since the project uses C++20.

include/mqt-core/qdmi/devices/na/Device.hpp (4)

18-18: LGTM!

Correctly includes the new SingletonDevice base class header.

---

31-32: LGTM!

The CRTP inheritance pattern (SingletonDevice<Device>) is the idiomatic approach for implementing a generic singleton base. The friend declaration is necessary to allow the base class to construct the singleton instance.

---

75-76: LGTM!

Deleting move operations is correct for singleton semantics—singletons should be neither copyable nor movable. This aligns with the dd/Device.hpp pattern shown in the relevant code snippets.

---

82-82: LGTM!

The override specifier is correct since SingletonDevice has a virtual destructor, and = default is appropriate here.

src/qdmi/devices/sc/CMakeLists.txt (1)

26-28: Path updates align with new directory structure.

The updated include paths and base directory references correctly reflect the new qdmi/devices/ layout. The addition of MQT::CoreQDMIBaseDevice as a private link dependency (line 137) properly connects the SC device to the new singleton base class.

Also applies to: 96-99, 130-130, 137-137, 180-180

src/qdmi/devices/na/Device.cpp (2)

15-18: Include path updates are correct.

The include paths now correctly reference the new qdmi/devices/na/ directory structure.

---

643-649: Singleton access with exception handling looks correct.

The try/catch pattern properly maps uninitialized device state to QDMI_ERROR_BADSTATE, consistent with the new singleton API contract.

include/mqt-core/qdmi/devices/dd/Device.hpp (3)

20-20: Singleton base class inheritance correctly implemented.

The Device class now properly derives from SingletonDevice<Device> using the CRTP pattern. The friend class SingletonDevice declaration correctly grants the base class access to the private constructor, which is required for the singleton factory method.

Also applies to: 35-36

---

77-77: Destructor override specifier is correct.

Adding override ensures the destructor properly overrides the virtual destructor in SingletonDevice<Device>.

---

248-250: Job constructor correctly uses pointer-based singleton access.

The change from Device::get().generateUniqueID() to Device::get()->generateUniqueID() correctly adapts to the new API that returns shared_ptr<Device>.

src/qdmi/devices/dd/Device.cpp (2)

15-15: Include path update is correct.

The include path now correctly references the new qdmi/devices/dd/ directory structure.

---

507-519: Unguarded Device::get() calls inside async lambda may cause termination.

Within std::async lambdas, if Device::get() throws (e.g., device finalized mid-execution), the exception will propagate out of the async task and cause std::terminate() when the future is destroyed or accessed. Consider wrapping these calls in try/catch or verifying the device remains initialized for the duration of job execution.

This is a theoretical concern assuming the device lifecycle is managed correctly (device won't be finalized while jobs are running). Please verify this invariant is enforced elsewhere, or consider adding defensive try/catch blocks:

     jobHandle_ = std::async(std::launch::async, [this]() {
+      try {
         qdmi::dd::Device::get()->increaseRunningJobs();
         // ... rest of job execution ...
         qdmi::dd::Device::get()->decreaseRunningJobs();
+      } catch (...) {
+        status_.store(QDMI_JOB_STATUS_FAILED);
+      }
     });

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider avoiding raw owning pointers for instance and mutex

You intentionally leak the mutex to sidestep static deinitialization order, and manage instance via a raw pointer:

MQT_HIDDEN inline static std::shared_ptr<ConcreteType>* instance = nullptr;
MQT_HIDDEN inline static std::mutex* mutex = new std::mutex;

This works, but it is non‑idiomatic and easy to misuse. A cleaner alternative is to rely on function‑local statics for initialization order and avoid manual new/delete:

static auto instanceStorage() -> std::shared_ptr<ConcreteType>& {
  static std::shared_ptr<ConcreteType> inst{};
  return inst;
}
static auto instanceMutex() -> std::mutex& {
  static std::mutex m;
  return m;
}

and then use instanceStorage() / instanceMutex() in initialize(), finalize(), and get(). That eliminates raw owning pointers and the intentional leak while still avoiding the static deinit‑order fiasco.

Not mandatory for correctness once get() is fixed, but it would simplify reasoning and static‑analysis tooling.

Also applies to: 32-35

🤖 Prompt for AI Agents

In include/mqt-core/qdmi/devices/base/Device.hpp around lines 22-27 (and
similarly 32-35), replace the raw owning pointers and intentional leak for
instance and mutex with function-local statics: create a function returning a
reference to a static std::shared_ptr<ConcreteType> (instanceStorage()) and
another returning a reference to a static std::mutex (instanceMutex()), then
update initialize(), finalize(), and get() to use those accessors instead of the
pointer variables; this removes manual new/delete, avoids the leak, and
preserves safe initialization order without changing external behavior.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find and read the Device.hpp file to see the full SingletonDevice implementation
find . -name "Device.hpp" -path "*/qdmi/devices/base/*" | head -5

Repository: munich-quantum-toolkit/core

Length of output: 118

---

🏁 Script executed:

# Read the full Device.hpp file to understand the complete SingletonDevice implementation
cat -n ./include/mqt-core/qdmi/devices/base/Device.hpp

Repository: munich-quantum-toolkit/core

Length of output: 3837

---

🏁 Script executed:

# Search for calls to SingletonDevice::get() to see if there are production callers
# and if they provide any guards or assumptions
rg "\.get\(\)" --type cpp -B 2 -A 2 | head -80

Repository: munich-quantum-toolkit/core

Length of output: 5521

---

🏁 Script executed:

# Search more specifically for SingletonDevice class usage and get() calls
# First find all files that use SingletonDevice
rg "SingletonDevice" --type cpp -l | head -20

Repository: munich-quantum-toolkit/core

Length of output: 248

---

🏁 Script executed:

# Check one concrete device implementation to see the pattern
cat -n ./include/mqt-core/qdmi/devices/dd/Device.hpp | head -50

Repository: munich-quantum-toolkit/core

Length of output: 1665

---

🏁 Script executed:

# Search for Device::get() or similar get() calls in QDMI device code
rg "Device::get|device.*get\(\)" include/mqt-core/qdmi/ src/qdmi/ --type cpp -B 2 -A 2

Repository: munich-quantum-toolkit/core

Length of output: 5176

---

🏁 Script executed:

# Check one of the Device.cpp implementation files to see the full context
cat -n ./src/qdmi/devices/dd/Device.cpp | head -100

Repository: munich-quantum-toolkit/core

Length of output: 4468

---

🏁 Script executed:

# Look at the specific get() calls and their exception handling
rg -A 10 "Device::get\(\)->" src/qdmi/devices/dd/Device.cpp | head -60

Repository: munich-quantum-toolkit/core

Length of output: 2340

---

🏁 Script executed:

# Let me check if there are any test files that verify get() behavior when device is not initialized
find . -path "*/test/*" -name "*device*test*.cpp" -o -path "*/test/*" -name "*Device*test*.cpp" | grep -i qdmi | head -5

Repository: munich-quantum-toolkit/core

Length of output: 166

---

🏁 Script executed:

# Check one test file to see initialization patterns
cat -n ./test/qdmi/devices/dd/device_status_test.cpp | head -80

Repository: munich-quantum-toolkit/core

Length of output: 3380

---

🏁 Script executed:

# Check the SessionGuard implementation to see if it calls initialize()
cat -n ./test/qdmi/dd/helpers/test_utils.hpp | head -100

Repository: munich-quantum-toolkit/core

Length of output: 140

---

🏁 Script executed:

# Find test_utils.hpp
find . -name "test_utils.hpp" -path "*/qdmi/*"

Repository: munich-quantum-toolkit/core

Length of output: 116

---

🏁 Script executed:

# Read test_utils.hpp to see SessionGuard implementation
cat -n ./test/qdmi/devices/dd/helpers/test_utils.hpp

Repository: munich-quantum-toolkit/core

Length of output: 2608

---

🏁 Script executed:

# Find the SessionGuard implementation
rg "SessionGuard::" test/qdmi/devices/dd/helpers/ -A 5

Repository: munich-quantum-toolkit/core

Length of output: 1020

---

Fix SingletonDevice::get() null pointer dereference and inconsistent error handling

The get() method has a critical undefined behavior bug: if instance is nullptr (before initialize() or after finalize()), the line assert(*instance != nullptr) dereferences a null pointer before the assertion fires. Additionally, production code in Device.cpp wraps some get() calls in try-catch blocks expecting std::runtime_error, but the implementation uses assert() which doesn't throw.

[[nodiscard]] static auto get() -> std::shared_ptr<ConcreteType> {
  const std::scoped_lock lock(*mutex);
  assert(*instance != nullptr &&
         "Device not initialized. Call `initialize()` first.");
  return *instance;
}

Should be changed to:

[[nodiscard]] static auto get() -> std::shared_ptr<ConcreteType> {
  const std::scoped_lock lock(*mutex);
  if (instance == nullptr || !*instance) {
    throw std::runtime_error(
        "Device not initialized. Call `initialize()` before `get()`.");
  }
  return *instance;
}

This matches the documented error handling pattern already expected by production code callers (lines 57-64, 71-85, 90-95 in Device.cpp which catch std::runtime_error), ensures all code paths are safe, and removes undefined behavior.

🤖 Prompt for AI Agents

In include/mqt-core/qdmi/devices/base/Device.hpp around lines 32 to 35, the
SingletonDevice::get() currently dereferences instance via assert(*instance !=
nullptr) causing undefined behavior if instance is null and uses assert instead
of throwing; change the implementation to lock the mutex, check if instance is
nullptr or *instance is null, and if so throw a std::runtime_error with the
message "Device not initialized. Call `initialize()` before `get()`.", otherwise
return *instance so callers expecting exceptions (not assertions) behave
correctly and no null pointer is dereferenced.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Double‑check EXPORT_NAME source and PIC property for INTERFACE target

Two small CMake nits in this block:

• EXPORT_NAME Core${ARG_ALIAS_NAME} assumes ARG_ALIAS_NAME is set in the including scope; if not, the export name degrades to just Core. Consider either hard‑coding the intended export name for this target or wiring a clearly defined argument/variable in the caller.
• For an INTERFACE library, INTERFACE_POSITION_INDEPENDENT_CODE is typically the property that propagates PIC requirements to consumers; setting POSITION_INDEPENDENT_CODE on the INTERFACE target itself may not have the intended effect.

You might want something along the lines of:

set_target_properties(${TARGET_NAME} PROPERTIES
  EXPORT_NAME CoreQDMIBaseDevice
  INTERFACE_POSITION_INDEPENDENT_CODE ON
)

(adjust export name to whatever is desired if ARG_ALIAS_NAME is indeed intentional).

🤖 Prompt for AI Agents

In src/qdmi/devices/base/CMakeLists.txt around lines 18 to 33, the EXPORT_NAME
uses ARG_ALIAS_NAME which may be undefined in the including scope and
POSITION_INDEPENDENT_CODE is being set on an INTERFACE target; replace or
hard‑code EXPORT_NAME (e.g., CoreQDMIBaseDevice) or ensure ARG_ALIAS_NAME is
explicitly defined/passed by the caller, and change the PIC property to
INTERFACE_POSITION_INDEPENDENT_CODE ON so PIC propagates to consumers; update
the set_target_properties call accordingly and verify the export name and PIC
behavior in the consuming projects.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing <stdexcept> include for std::runtime_error.

The catch clause references std::runtime_error, but no header providing it is directly included.

Add at the top of the file with other includes:

 #include <utility>
+#include <stdexcept>

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 GitHub Check: 🇨‌ Lint / 🚨 Lint

[warning] 360-360: src/qdmi/devices/dd/Device.cpp:360:23 [misc-include-cleaner]
no header providing "std::runtime_error" is directly included

🤖 Prompt for AI Agents

In src/qdmi/devices/dd/Device.cpp around lines 357 to 363, the catch uses
std::runtime_error but the header <stdexcept> is not included; add an include
for <stdexcept> at the top of the file with the other includes (so the catch
compiles and the symbol is resolved).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Empty catch block silently swallows exceptions.

Same issue as in the NA device implementation. Consider adding a comment or parameter name to document the intentional silence.

Apply this diff:

 void MQT_DDSIM_QDMI_device_session_free(MQT_DDSIM_QDMI_Device_Session session) {
   try {
     // Use the new get() that returns a shared_ptr.
     qdmi::dd::Device::get()->sessionFree(session);
-  } catch (const std::runtime_error&) {
+  } catch (const std::runtime_error& /*unused*/) {
+    // Device not initialized; session cannot be freed - silently ignore.
   }
 }

🧰 Tools

🪛 GitHub Check: 🇨‌ Lint / 🚨 Lint

[warning] 826-826: src/qdmi/devices/dd/Device.cpp:826:5 [bugprone-empty-catch]
empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether

🤖 Prompt for AI Agents

In src/qdmi/devices/dd/Device.cpp around lines 823 to 828, the empty catch block
silently swallows std::runtime_error; update the catch to either name the
exception (e.g., catch (const std::runtime_error& e)) and add a brief comment
explaining why the exception is intentionally ignored, or log the exception at
debug/trace level so the silence is documented and future debugging is possible;
ensure the chosen approach matches the NA device implementation for consistency.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing <stdexcept> include for std::runtime_error.

The catch clause references std::runtime_error, but no header providing it is directly included. Add the missing include to satisfy include-what-you-use requirements.

Add at the top of the file with other includes:

 #include <vector>
+#include <stdexcept>

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 GitHub Check: 🇨‌ Lint / 🚨 Lint

[warning] 228-228: src/qdmi/devices/na/Device.cpp:228:23 [misc-include-cleaner]
no header providing "std::runtime_error" is directly included

🤖 Prompt for AI Agents

In src/qdmi/devices/na/Device.cpp around lines 225 to 231, the catch block
references std::runtime_error but the file does not include the <stdexcept>
header; add #include <stdexcept> alongside the other includes at the top of the
file so the symbol is declared and the code satisfies include-what-you-use.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix void return and empty catch block.

Two issues flagged by static analysis:

1. Line 662: return statement within a void function should not have a specified return value. sessionFree returns void, so return should be plain.
2. Line 663: Empty catch block silently swallows exceptions. Consider adding a comment explaining intentional silence or logging at debug level.

Apply this diff:

 void MQT_NA_QDMI_device_session_free(MQT_NA_QDMI_Device_Session session) {
   try {
     // Use the new get() that returns a shared_ptr.
-    return qdmi::na::Device::get()->sessionFree(session);
-  } catch (const std::runtime_error&) {
+    qdmi::na::Device::get()->sessionFree(session);
+  } catch (const std::runtime_error& /*unused*/) {
+    // Device not initialized; session cannot be freed - silently ignore.
   }
 }

🧰 Tools

🪛 GitHub Check: 🇨‌ Lint / 🚨 Lint

[warning] 663-663: src/qdmi/devices/na/Device.cpp:663:5 [bugprone-empty-catch]
empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether

---

[warning] 662-662: src/qdmi/devices/na/Device.cpp:662:5 [readability-avoid-return-with-void-value]
return statement within a void function should not have a specified return value

🤖 Prompt for AI Agents

In src/qdmi/devices/na/Device.cpp around lines 660 to 664, the code uses `return
qdmi::na::Device::get()->sessionFree(session);` inside a void function and has
an empty catch that swallows exceptions; change the call to invoke
`sessionFree(session)` without returning a value (call the method then plain
`return;` or simply fall through) and replace the empty catch block with a
deliberate no-op comment or a debug-level log (e.g., comment "// intentionally
ignoring errors" or a debug log call) so exceptions are not silently swallowed
without explanation.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider removing unused <atomic> include.

With the singleton management moved to the SingletonDevice base class, <atomic> appears to no longer be used in this file. Consider removing it for cleaner dependencies.

-#include <atomic>

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

#include <atomic>

🤖 Prompt for AI Agents

In src/qdmi/devices/sc/Device.cpp around line 21, the file still includes
<atomic> but that header is no longer used after moving singleton management to
SingletonDevice; remove the #include <atomic> line, run a full rebuild or at
least compile this translation unit to ensure there are no remaining references,
and commit the cleaned include removal.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider adding a comment for the empty catch block.

The empty catch block is intentional for an idempotent free operation, but a brief comment would clarify this design choice and prevent future confusion.

   try {
     // Use the new get() that returns a shared_ptr.
     qdmi::sc::Device::get()->sessionFree(session);
   } catch (const std::runtime_error&) {
+    // Silently ignore: if device is uninitialized, there's nothing to free.
   }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	try {
	// Use the new get() that returns a shared_ptr.
	qdmi::sc::Device::get()->sessionFree(session);
	} catch (const std::runtime_error&) {
	}
	try {
	// Use the new get() that returns a shared_ptr.
	qdmi::sc::Device::get()->sessionFree(session);
	} catch (const std::runtime_error&) {
	// Silently ignore: if device is uninitialized, there's nothing to free.
	}

🤖 Prompt for AI Agents

In src/qdmi/devices/sc/Device.cpp around lines 492 to 496, the empty catch block
swallowing std::runtime_error should include a brief inline comment clarifying
it's intentional; update the catch to contain a one-line comment such as
"Intentionally ignored — sessionFree is idempotent and may throw if session
already freed" so future readers understand the design choice and no behavior
changes are made.

[burgholzer]

Some of the changes here have already made their way into main through #1355. The key take-away for me from here was that we intentionally leak memory to avoid the fiasco. This led to a simpler version than implemented here, where we, again, simply use a static variable in the singleton getter. Since C++11, static variable initialization is guaranteed to be thread safe.
There are still some nice further improvements in this PR that definitely make sense. I am not sure if it is easier to rebase this PR or to simply set up a new one with the best practices.