add CodeQL, Trivy and Clippy checks by bratpiorka · Pull Request #48 · oneapi-src/oneapi-rs

bratpiorka · 2026-09-14T08:48:50Z

add CodeQL, Trivy and Clippy checks

Copilot

🔵 Needs a closer look

Trivy failures do not gate deployment, and CodeQL does not analyze Rust.

Pull request overview

Adds CodeQL, Trivy, and Clippy checks to the CI workflow.

Changes:

Runs Clippy with warnings denied.
Adds C++ CodeQL analysis.
Adds Trivy filesystem security scanning.

File summaries

File	Summary
`.github/workflows/basic.yml`	Integrates the new checks; Trivy does not gate documentation deployment, and CodeQL omits Rust analysis.

Review details

Suppressed comments (2)

.github/workflows/basic.yml:136

A failed Trivy scan does not currently block documentation deployment: deploy-documentation still depends only on build-and-test, so a finding causes this job to fail while the main-branch deployment can proceed. Add trivy to the deployment dependencies (or otherwise gate the deployment) so the new exit-code: 1 check protects the published artifact.

    if: (github.event_name == 'push' && github.ref == 'refs/heads/main') || (github.event_name == 'workflow_dispatch' && inputs.deploy_documentation == 'true')

.github/workflows/basic.yml:64

languages: cpp leaves the Rust workspace completely outside this CodeQL check, even though the repository is primarily Rust and the pinned CodeQL action supports Rust analysis. As a result, vulnerabilities in the public Rust API and bindings are not reported; include rust as an analyzed language (or add a separate Rust matrix job).

        languages: cpp

Files reviewed: 1/1 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

szymon-zadworny

I don't have any issues with additional safety comments, although it was previously decided that we won't document -sys crates.

However, one comment change is dangerous and should be reverted or updated.

szymon-zadworny · 2026-09-14T13:00:04Z

+/// # Safety
+///
+/// `ptr` must come from `Arc::into_raw` and represent a strong reference owned by this callback.


This wording removes the information that the reference count needs to be incremented by the caller. This makes the whole comment pointless.

szymon-zadworny · 2026-09-14T13:02:29Z

            NdRange::new([1024], [16]),
            &kernel,
-            (f16::from_f32(3.14), &mut device_array),
+            (f16::from_f32(std::f32::consts::PI), &mut device_array),


f16 provides a PI constant.
https://docs.rs/half/latest/half/struct.f16.html#associatedconstant.PI

szymon-zadworny · 2026-09-14T13:05:06Z

+impl Default for Queue {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+


We deemed this to be dangerous w.r.t. performance since it hides the device the Queue is being assigned to.

ok will remove this

szymon-zadworny · 2026-09-14T13:08:07Z

+impl Default for SharedWaker {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+


This code will never be used.

this is a fix for warning

warning: you should consider adding a `Default` implementation for `SharedWaker` --> sycl/sycl-rs-sys/src/types-sys.rs:19:5 | 19 | / pub fn new() -> Self { 20 | | Self { 21 | | waker: AtomicWaker::new(), 22 | | done: AtomicBool::new(false), 23 | | } 24 | | } | |_____^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#new_without_default = note: `#[warn(clippy::new_without_default)]` on by default help: try adding this | 18 + impl Default for SharedWaker { 19 + fn default() -> Self { 20 + Self::new() 21 + } 22 + } |

szymon-zadworny · 2026-09-14T13:13:59Z

 // SPDX-License-Identifier: MIT OR Apache-2.0
 //

+#[allow(clippy::missing_safety_doc)]


Is this issue the reason behind that attribute? If yes - it should be documented with a comment.

szymon-zadworny · 2026-09-14T13:14:45Z


 use crate::types::SharedWaker;

+#[allow(clippy::missing_safety_doc)]


Is this issue the reason behind that attribute? If yes - it should be documented with a comment.

this a fix for

warning: unsafe function's docs are missing a `# Safety` section --> sycl/sycl-rs-sys/src/event-sys.rs:13:1 | 13 | #[cxx::bridge(namespace = "sycl_shims::event")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#missing_safety_doc

These warnings are incorrectly emitted due to the linked issue. This allow attribute is a workaround.

bratpiorka · 2026-09-15T13:45:06Z

    ];

-    cxx_build::bridges(&rust_sources)
+    cxx_build::bridges(rust_sources)


fixes:

warning: the borrowed expression implements the required traits warning: the borrowed expression implements the required traits

bratpiorka · 2026-09-15T13:46:02Z


        // Set the callback on first Future poll (Futures can't be active until polled)
-        if *this.set_callback == false {
+        if !*this.set_callback {


fixes:

warning: equality checks against false can be replaced by a negation --> sycl/sycl-rs/src/event.rs:63:12 | 63 | if *this.set_callback == false { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!*this.set_callback` | = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#bool_comparison = note: `#[warn(clippy::bool_comparison)]` on by default

bratpiorka · 2026-09-15T13:46:54Z

    /// Submits a barrier to the queue.
    pub fn barrier(&mut self) -> Result<Event> {
-        self.barrier_with_deps(&[]).map(Into::into)
+        self.barrier_with_deps(&[])


fixes:

warning: useless conversion to the same type: `event::Event` --> sycl/sycl-rs/src/queue.rs:131:36 | 131 | self.barrier_with_deps(&[]).map(Into::into) | ____________________________________-^^^^^^^^^^^^^^^ 132 | | } | |____- help: consider removing | = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#useless_conversion = note: `#[warn(clippy::useless_conversion)]` on by default

bratpiorka · 2026-09-15T13:47:21Z

    pub(crate) unsafe fn new(allocator: A, len: usize) -> Self {
        let layout = Layout::array::<T>(len).unwrap();
-        let ptr = match allocator.allocate(layout.clone()) {
+        let ptr = match allocator.allocate(layout) {


fixes:

warning: using `clone` on type `Layout` which implements the `Copy` trait --> sycl/sycl-rs/src/usmbox.rs:54:44 | 54 | let ptr = match allocator.allocate(layout.clone()) { | ^^^^^^^^^^^^^^ help: try removing the `clone` call: `layout` | = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#clone_on_copy = note: `#[warn(clippy::clone_on_copy)]` on by default warning: `sycl-rs` (lib test) generated 18 warnings (13 duplicates) (run `cargo clippy --fix --lib -p sycl-rs --tests` to apply 1 suggestion) warning: `sycl-rs` (lib) generated 18 warnings (5 duplicates) (run `cargo clippy --fix --lib -p sycl-rs` to apply 2 suggestions)

bratpiorka · 2026-09-15T13:50:26Z

    ];

-    cxx_build::bridges(&rust_sources)
+    cxx_build::bridges(rust_sources)


fixes:

warning: the borrowed expression implements the required traits

szymon-zadworny · 2026-09-16T13:28:16Z

            NdRange::new([1024], [16]),
            &kernel,
-            (f16::from_f32(3.14), &mut device_array),
+            (std::f16::consts::PI, &mut device_array),


This project uses half::f16 because std::f16 hasn't been stabilized yet - it's only available on nightly toolchains.

Removing the std:: prefix will fix this.

Copilot

🟢 Approval recommended

Only minor documentation nits were identified; no blocking issues remain.

Review details

Files reviewed: 17/17 changed files
Comments generated: 3
Review effort level: Lite

Copilot

🟢 Approval recommended

The reviewed changes have no unresolved approval-blocking issues.

Review details

Files reviewed: 15/15 changed files
Comments generated: 0 new
Review effort level: Lite

szymon-zadworny

lgtm

bratpiorka requested review from szymon-zadworny and a lite review from Copilot September 14, 2026 08:48

Copilot started reviewing on behalf of bratpiorka September 14, 2026 08:49 View session

bratpiorka force-pushed the rrudnick_sdl branch from 7bd06e1 to 81b1e14 Compare September 14, 2026 08:51

Copilot AI reviewed Sep 14, 2026

View reviewed changes

bratpiorka force-pushed the rrudnick_sdl branch 5 times, most recently from 4101444 to 497dfd3 Compare September 14, 2026 12:50

szymon-zadworny reviewed Sep 14, 2026

View reviewed changes

szymon-zadworny self-requested a review September 14, 2026 13:16

szymon-zadworny requested changes Sep 14, 2026

View reviewed changes

bratpiorka force-pushed the rrudnick_sdl branch from 497dfd3 to 2a24938 Compare September 14, 2026 13:53

bratpiorka commented Sep 15, 2026

View reviewed changes

bratpiorka force-pushed the rrudnick_sdl branch 4 times, most recently from e77035a to 07802e0 Compare September 16, 2026 12:49

bratpiorka requested a review from szymon-zadworny September 16, 2026 12:49

szymon-zadworny requested changes Sep 16, 2026

View reviewed changes

bratpiorka force-pushed the rrudnick_sdl branch 2 times, most recently from 213215c to c8c2203 Compare September 16, 2026 18:49

bratpiorka requested review from szymon-zadworny and a lite review from Copilot September 16, 2026 19:19

Copilot started reviewing on behalf of bratpiorka September 16, 2026 19:20 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread sycl/sycl-rs-sys/src/event-sys.rs Outdated

Comment thread sycl/sycl-rs-sys/src/types-sys.rs Outdated

Comment thread sycl/sycl-rs/src/queue.rs Outdated

bratpiorka force-pushed the rrudnick_sdl branch 2 times, most recently from 997decf to 840c048 Compare September 17, 2026 07:10

bratpiorka requested a lite review from Copilot September 17, 2026 07:11

Copilot started reviewing on behalf of bratpiorka September 17, 2026 07:11 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

bratpiorka added 2 commits September 17, 2026 08:11

add CodeQL, Trivy and Clippy checks

330a30a

Fix Clippy findings

840c048

szymon-zadworny approved these changes Sep 17, 2026

View reviewed changes

szymon-zadworny merged commit 6b0a436 into oneapi-src:main Sep 17, 2026
6 checks passed


		use crate::types::SharedWaker;

		#[allow(clippy::missing_safety_doc)]

Conversation

bratpiorka commented Sep 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

szymon-zadworny left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bratpiorka Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

szymon-zadworny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

szymon-zadworny left a comment •

edited

Loading

bratpiorka Sep 15, 2026 •

edited

Loading