Skip to content

Bump LTO shared lib for C API - #411

Merged
ahuber21 merged 6 commits into
mainfrom
dev/eglaser-bump-c-api-lib
Oct 7, 2026
Merged

ahuber21 merged 6 commits into
mainfrom
dev/eglaser-bump-c-api-lib

Conversation

@ethanglaser

Copy link
Copy Markdown
Member

No description provided.

@ethanglaser
ethanglaser marked this pull request as ready for review October 6, 2026 14:55
@ethanglaser

Copy link
Copy Markdown
Member Author

LGTM!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Returning the dataset by value introduces a full, memory-intensive copy during index conversion.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates C API index conversion and points GCC 11.2 builds to the latest LTO archive.

Changes:

  • Returns simple datasets directly during conversion.
  • Updates the nightly LTO shared-library URL.
File Description
bindings/​c/​src/​data_builder/​simple.hpp Changes simple dataset access during conversion.
bindings/​c/​CMakeLists.txt Bumps the LTO archive URL.

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

Comment thread bindings/c/src/data_builder/simple.hpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The changes correctly avoid extra copies, preserve temporary lifetimes, and reference an available archive.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@ahuber21
ahuber21 merged commit 86e40e9 into main Oct 7, 2026
24 checks passed
@ahuber21
ahuber21 deleted the dev/eglaser-bump-c-api-lib branch October 7, 2026 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants