feat: C++ bindings support by HideBa · Pull Request #41 · cityjson/flatcitybuf

HideBa · 2026-02-06T21:08:47Z

No description provided.

Copilot

Pull request overview

Adds a new fcb_cpp Rust crate and accompanying CMake/C++ scaffolding to expose FlatCityBuf core read/write capabilities to C++ consumers via the cxx bridge.

Changes:

Introduces Rust CXX bindings for reading (metadata + iteration + bbox query) and writing FCB files.
Adds CMake project + C++ example demonstrating usage and linking.
Wires the new crate into the Rust workspace and adds cxx / cxx-build workspace dependencies.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`src/rust/fcb_cpp/src/lib.rs`	Defines the `#[cxx::bridge]` API surface for C++ consumers.
`src/rust/fcb_cpp/src/reader.rs`	Implements reader + iterator wrappers intended for C++ interop.
`src/rust/fcb_cpp/src/writer.rs`	Implements a writer wrapper for C++ interop.
`src/rust/fcb_cpp/build.rs`	Builds CXX bridge code as part of the Rust build.
`src/rust/fcb_cpp/Cargo.toml`	Declares the new `fcb_cpp` staticlib crate and deps.
`src/rust/fcb_cpp/README.md`	Documents build steps and the C++ API.
`src/rust/Cargo.toml`	Adds `fcb_cpp` to workspace members and adds `cxx` deps.
`src/cpp/CMakeLists.txt`	Adds a CMake build that generates/compiles the CXX bridge and links Rust.
`src/cpp/examples/example.cpp`	Provides a sample executable using the new C++ API.
`.vscode/settings.json`	Points VS Code CMake integration at `src/cpp`.
`.gitignore`	Ignores `build/` and fixes `*.so` ignore pattern formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T21:15:45Z

src/rust/fcb_cpp/src/reader.rs

+    FcbMetadata {
+        version: 1, // TODO: Extract from header when available


version is hard-coded to 1, but the FCB header appears to carry a version string (header.version()). Returning an incorrect version is misleading for API consumers; parse/populate this field from the header instead of hard-coding it.

Suggested change

FcbMetadata {

version: 1, // TODO: Extract from header when available

// Derive version from the header; fall back to 1 if parsing fails

let version = header

.version()

.parse()

.unwrap_or(1);

FcbMetadata {

version,

Copilot · 2026-02-06T21:15:45Z

src/rust/fcb_cpp/src/reader.rs

+// We're using single-threaded access, so this is safe for our use case.
+// C++ bindings will be called from a single thread.
+unsafe impl Send for IteratorWrapper {}
+


Avoid unsafe impl Send here unless it’s strictly necessary and you can uphold Send’s thread-safety guarantees. IteratorWrapper appears to be composed of Send types already, so the unsafe impl is likely redundant; consider removing the + Send bound on the trait object and/or rely on the compiler’s auto-traits instead of forcing Send unsafely.

Copilot · 2026-02-06T21:15:45Z

src/rust/fcb_cpp/src/writer.rs

+/// Create a new FCB writer with CityJSON metadata
+pub fn fcb_writer_new(metadata_json: &str) -> Result<Box<FcbFileWriter>, String> {
+    let cj: CityJSON = serde_json::from_str(metadata_json)
+        .map_err(|e| format!("Failed to parse CityJSON metadata: {}", e))?;


The CXX bridge declares this as returning Result<...> (i.e., cxx::Result), but the implementation returns Result<..., String>. Align the signature with the bridge by returning cxx::Result<...> and mapping errors into cxx::Error/cxx::Exception instead of String.

Copilot · 2026-02-06T21:15:45Z

src/rust/fcb_cpp/src/writer.rs

+pub struct FcbFileWriter {
+    cj_metadata: CityJSON,
+    features: Vec<CityJSONFeature>,
+}


This wrapper buffers all added features in memory (Vec<CityJSONFeature>) until write(). For large datasets this can cause high peak memory usage for C++ callers. If possible, consider a streaming design (open writer with a path/handle, add_feature writes immediately to the internal temp buffer) or at least document/guard against unbounded growth.

Copilot · 2026-02-06T21:15:46Z

src/rust/fcb_cpp/build.rs

+    cxx_build::bridge("src/lib.rs")
+        .flag_if_supported("-std=c++17")
+        .compile("fcb_cpp");
+


This build script compiles the CXX bridge C++ code into the Rust staticlib via cxx_build::bridge(...).compile(...), but the CMake build also generates and compiles lib.rs.cc and links it alongside libfcb_cpp.a. This can result in duplicate symbol definitions at link time. Choose one approach: either compile/link the generated .cc only on the C++ side (and drop the cxx_build::bridge().compile(...) step), or rely on the Rust build output and avoid compiling the generated .cc again in CMake.

Suggested change

cxx_build::bridge("src/lib.rs")

.flag_if_supported("-std=c++17")

.compile("fcb_cpp");

Copilot · 2026-02-06T21:15:46Z

src/rust/fcb_cpp/src/reader.rs

+unsafe impl Send for DirectIteratorHelper {}
+


Same concern as above: unsafe impl Send for DirectIteratorHelper should be avoided unless you can prove it is sound. If this is only needed to satisfy the Box<dyn ... + Send> field, prefer removing the Send requirement rather than adding an unsafe impl.

Copilot · 2026-02-06T21:15:47Z

src/rust/fcb_cpp/src/writer.rs

+    let mut fcb_writer = FcbWriter::new(writer.cj_metadata.clone(), None, None, None)
+        .map_err(|e| format!("Failed to create FCB writer: {}", e))?;
+
+    for feature in &writer.features {


writer is consumed by this function, but writer.cj_metadata.clone() clones potentially large metadata unnecessarily. Prefer moving cj_metadata out of the boxed writer (e.g., by destructuring *writer) so the metadata can be transferred into FcbWriter::new without cloning.

Suggested change

let mut fcb_writer = FcbWriter::new(writer.cj_metadata.clone(), None, None, None)

.map_err(|e| format!("Failed to create FCB writer: {}", e))?;

for feature in &writer.features {

let FcbFileWriter { cj_metadata, features } = *writer;

let mut fcb_writer = FcbWriter::new(cj_metadata, None, None, None)

.map_err(|e| format!("Failed to create FCB writer: {}", e))?;

for feature in &features {

Copilot · 2026-02-06T21:15:47Z

src/cpp/CMakeLists.txt

+add_custom_command(
+    OUTPUT ${GENERATED_DIR}/lib.rs.cc
+    COMMAND ${CXXBRIDGE} ${RUST_LIB_DIR}/fcb_cpp/src/lib.rs > ${GENERATED_DIR}/lib.rs.cc
+    DEPENDS ${RUST_LIB_DIR}/fcb_cpp/src/lib.rs
+    COMMENT "Generating C++ source from CXX bridge"
+)
+
+# Create fcb library
+add_library(fcb STATIC
+    ${GENERATED_DIR}/lib.rs.cc
+)


CMake generates and compiles lib.rs.cc (the CXX bridge source) and links it with the Rust staticlib, while the Rust crate’s build.rs also compiles the same bridge code via cxx_build::bridge(...).compile(...). This duplication is likely to cause duplicate symbol errors. Adjust either the Rust build or the CMake build so the bridge .cc is compiled exactly once in the final link.

Suggested change

add_custom_command(

OUTPUT ${GENERATED_DIR}/lib.rs.cc

COMMAND ${CXXBRIDGE} ${RUST_LIB_DIR}/fcb_cpp/src/lib.rs > ${GENERATED_DIR}/lib.rs.cc

DEPENDS ${RUST_LIB_DIR}/fcb_cpp/src/lib.rs

COMMENT "Generating C++ source from CXX bridge"

)

# Create fcb library

add_library(fcb STATIC

${GENERATED_DIR}/lib.rs.cc

)

# Note: The CXX bridge C++ source is compiled by the Rust build (build.rs via cxx_build),

# so we only generate the header here to avoid duplicate symbol definitions.

# Create fcb library (acts as a wrapper around the Rust static library)

add_library(fcb STATIC)

Copilot · 2026-02-06T21:15:47Z

src/rust/fcb_cpp/src/reader.rs

+    let iter = reader
+        .inner


select_all() on FcbReader takes self by value, but reader.inner.select_all() attempts to move inner out of a Box<FcbFileReader> through deref, which doesn’t compile. Destructure the boxed FcbFileReader to move out inner (or change the wrapper to store Option<FcbReader<_>> and take() it) before calling select_all().

Suggested change

let iter = reader

.inner

// Move the FcbFileReader out of the Box, then move out its inner FcbReader

let FcbFileReader { inner } = *reader;

let iter = inner

Copilot · 2026-02-06T21:15:47Z

src/cpp/CMakeLists.txt

+# Find cxxbridge CLI tool
+find_program(CXXBRIDGE cxxbridge)
+if(NOT CXXBRIDGE)
+    message(STATUS "cxxbridge not found, installing via cargo...")
+    execute_process(
+        COMMAND cargo install cxxbridge-cmd
+        RESULT_VARIABLE CXXBRIDGE_INSTALL_RESULT
+    )
+    find_program(CXXBRIDGE cxxbridge REQUIRED)
+endif()


This CMake script automatically installs and executes the cxxbridge-cmd tool via cargo install cxxbridge-cmd without pinning a version or verifying integrity, which introduces a build-time supply-chain risk. An attacker who compromises the crate or crates.io could ship a malicious cxxbridge-cmd that runs in your build and generates backdoored bridge code compiled into production artifacts. To mitigate this, avoid auto-installation in the build, pin cxxbridge-cmd to a specific trusted version or commit, and/or vendor or pre-generate the bridge code so that build steps do not fetch and execute mutable third-party tooling.

feat: C++ bindings support

b745340

HideBa requested a review from Copilot February 6, 2026 21:08

Copilot started reviewing on behalf of HideBa February 6, 2026 21:09 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: C++ bindings support#41

feat: C++ bindings support#41
HideBa wants to merge 1 commit intomainfrom
cpp_bindings

HideBa commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		FcbMetadata {
		version: 1, // TODO: Extract from header when available

-    FcbMetadata {
-        version: 1, // TODO: Extract from header when available
+    // Derive version from the header; fall back to 1 if parsing fails
+    let version = header
+        .version()
+        .parse()
+        .unwrap_or(1);
+    FcbMetadata {
+        version,

	cxx_build::bridge("src/lib.rs")
	.flag_if_supported("-std=c++17")
	.compile("fcb_cpp");

-    let iter = reader
-        .inner
+    // Move the FcbFileReader out of the Box, then move out its inner FcbReader
+    let FcbFileReader { inner } = *reader;
+    let iter = inner

Conversation

HideBa commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant