Skip to content

fix(cpp): build and repair asset::AssetContext - #580

Merged
sunli829 merged 2 commits into
mainfrom
fix/cpp-asset-context
Aug 26, 2026
Merged

fix(cpp): build and repair asset::AssetContext#580
sunli829 merged 2 commits into
mainfrom
fix/cpp-asset-context

Conversation

@sunli829

Copy link
Copy Markdown
Collaborator

The bug

longbridge.hpp has always included asset_context.hpp, so C++ users could see asset::AssetContext — but cpp/src/asset_context.cpp was never listed in cpp/CMakeLists.txt. The class was declared to users and then failed to link.

Because it was never built, three separate problems had accumulated behind the missing build entry.

1. lb_statement_item_t was not in the generated header

CStatementItem is only reachable through the void* async-result pointer, so cbindgen never emitted it and no C or C++ caller could read what lb_asset_context_statements returns. Exported it the same way the GridContext payloads already are.

CAssetContext was also missing from the rename map — every other context type was mapped, so the header exposed the raw Rust name while the C++ side forward-declares lb_asset_context_t:

// before
const struct CAssetContext *lb_asset_context_new(const struct lb_config_t *config);
// after
typedef struct lb_asset_context_t lb_asset_context_t;
const struct lb_asset_context_t *lb_asset_context_new(const struct lb_config_t *config);

2. asset_context.cpp did not include the C header

Every other context source includes longbridge.h. This one included only its own .hpp, which forward-declares lb_asset_context_t as an opaque struct and nothing else — so lb_asset_context_retain and friends were undeclared:

error: 'lb_asset_context_retain' was not declared in this scope

3. statement_download_url decoded the wrong shape

It read res->data as a lb_statement_download_url_response_t* and took ->url. No such type exists anywhere in the C layer. lb_asset_context_download_url resolves a CString, so the callback receives the URL as a bare const char* — the same convention as QuoteContext::quote_level:

-  auto resp = (const lb_statement_download_url_response_t*)res->data;
-  StatementDownloadUrlResponse result;
-  result.url = resp->url;
+  StatementDownloadUrlResponse result;
+  result.url = (const char*)res->data;

Verification

  • asset_context.cpp.o now builds, and liblongbridge_cpp.so exports the asset::AssetContext symbols.
  • statements checked against a live environment: returns the same empty list the Rust core returns for the same account.
  • statement_download_url is compile- and link-checked only — the test account has no statement, so there is no file_key to exercise it with. It follows the same (const char*)res->data convention as quote_level, but this has not been confirmed against a real response.
  • cargo clippy --all --all-features and cargo +nightly fmt --all clean; full CMake build with 0 errors.

Notes

Independent of #579 — both branch off main and touch disjoint files, except that both add an entry at the top of CHANGELOG.md, so whichever merges second will need a trivial conflict resolution.

🤖 Generated with Claude Code

`longbridge.hpp` has always included `asset_context.hpp`, so C++ users
could see `asset::AssetContext` — but `cpp/src/asset_context.cpp` was
never listed in `cpp/CMakeLists.txt`, so the class was declared and then
failed to link. It had never compiled either; three separate problems had
accumulated behind the missing build entry.

**`lb_statement_item_t` was not in the generated header.**
`CStatementItem` is only reachable through the `void*` async-result
pointer, so cbindgen never emitted it and no caller could read what
`lb_asset_context_statements` returns. Exported it the same way the
GridContext payloads are. `CAssetContext` was also missing from the
rename map — every other context type was mapped, so the header exposed
the raw Rust name (`const struct CAssetContext *lb_asset_context_new(…)`)
while the C++ side forward-declares `lb_asset_context_t`.

**`asset_context.cpp` did not include the C header.** Every other context
source includes `longbridge.h`; this one included only its own `.hpp`,
which forward-declares `lb_asset_context_t` as an opaque struct and
nothing else — so `lb_asset_context_retain` and friends were undeclared.

**`statement_download_url` decoded the wrong shape.** It read `res->data`
as a `lb_statement_download_url_response_t*` and took `->url`. No such
type exists anywhere in the C layer: `lb_asset_context_download_url`
resolves a `CString`, so the callback receives the URL as a bare
`const char*` — the same convention as `QuoteContext::quote_level`.

Verified against a live environment: `statements` returns the same empty
list the Rust core returns for the same account, and the symbols are
present in `liblongbridge_cpp.so`. `statement_download_url` is compile-
and link-checked only — the test account has no statement to supply a
`file_key`.
sunli829 added a commit that referenced this pull request Aug 26, 2026
## The bug

`std::vector::data()` is allowed to return `nullptr` for an empty
vector, and that is exactly what the C++ binding passes when a list
argument is omitted. The C layer fed the pointer straight to
`std::slice::from_raw_parts`, which requires a non-null, aligned pointer
**even for a zero-length slice** — so the call was undefined behaviour
and aborted the process:

```
thread '<unnamed>' panicked at c/src/quote_context/context.rs:784:24:
unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be
aligned and non-null, and the total size of the slice not to exceed `isize::MAX`

This indicates a bug in the program. This Undefined Behavior check is optional,
and cannot be relied on for safety.
thread caused non-unwinding panic. aborting.
```

Reproduced live by calling `QuoteContext::warrant_list` from C++ with no
filters — the most natural way to call it:

```cpp
ctx.warrant_list("700.HK", WarrantSortBy::LastDone, SortOrderType::Descending,
                 {}, {}, {}, {}, {},   // <-- every filter omitted
                 [](auto res) { ... });
```

## The fix

All 17 `from_raw_parts` call sites had this shape, so they now go
through a single null-tolerant helper in `c/src/types/mod.rs`:

```rust
pub(crate) unsafe fn slice_from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
    if len == 0 || data.is_null() {
        &[]
    } else {
        std::slice::from_raw_parts(data, len)
    }
}
```

Covers `quote_context` (8 sites), `trade_context` (5), `agent_context`
(2), `alert_context` (1) and `cstr_array_to_rust` in `types` (1).

## Notes

- **Internal only.** The generated `longbridge.h` is byte-for-byte
unchanged, so the C and C++ public APIs are untouched.
- Release builds do not have the debug UB check, so the abort only shows
up in debug — but the undefined behaviour is there either way.
- Verified with `cargo clippy --all --all-features`, `cargo +nightly fmt
--all`, and a full CMake build.
- Independent of #580 — both branch off `main` and touch disjoint files,
except that both add an entry at the top of `CHANGELOG.md`, so whichever
merges second will need a trivial conflict resolution.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@sunli829

Copy link
Copy Markdown
Collaborator Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: sunli829 <20092316+sunli829@users.noreply.github.com>

Copilot AI commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved by merging origin/main into this branch and fixing the CHANGELOG.md conflict in commit 616a0119.

@sunli829
sunli829 merged commit adbd685 into main Aug 26, 2026
@sunli829
sunli829 deleted the fix/cpp-asset-context branch August 26, 2026 02:38
sunli829 added a commit that referenced this pull request Aug 26, 2026
`QuoteContext.warrant_list` failed outright whenever the server included a
placeholder row. On `700.HK` with no filters, 324 of the 746 warrants
returned are placeholders — only `symbol`, `name` and `last_done` carry a
value, everything else is an empty string:

```
FilterWarrant { symbol: "68463.HK", name: "HS#TENCTRC2702E",
                last_done: "0.244", expiry_date: "", strike_price: "",
                premium: "", …, status: 0 }
```

The decimal fields already had `.parse().unwrap_or_default()` fallbacks.
Two strict fields did not, and either one aborted the conversion of the
*entire* list:

- `expiry_date: ""` → `parse field: expiry_date: the 'year' component
  could not be parsed`
- `status: 0` → `No discriminant in enum WarrantStatus matches the value
  0` (the enum only had `Suspend = 2` / `PrepareList = 3` / `Normal = 4`)

## Breaking changes

`WarrantInfo.expiry_date` is now optional: Rust `Option<Date>`, Python
`Optional[date]`, Node.js `NaiveDate | null`, Java nullable `LocalDate`,
C `const lb_date_t*` (NULL when absent), C++ `std::optional<Date>`.

`WarrantStatus` gains an `Unknown` variant, placed first to match
`WarrantType`. An unrecognized discriminant now maps to it rather than
failing, so a future unknown status cannot take out the whole list again.
Adding a variant shifts the ordinal of the existing ones in the C, C++,
Java, Node.js and Python bindings.

Placeholder rows now come through as `expiry_date: None, status: Unknown`
instead of being dropped or aborting the call. Verified live from both
Rust and C++: 746 rows returned, 324 of them placeholders.

Also carries the two fixes already submitted separately against `main` as
#579 (null pointers for empty list arguments) and #580 (building and
repairing `asset::AssetContext`) — both were needed to exercise this
change from C++.
sunli829 added a commit that referenced this pull request Aug 26, 2026
`QuoteContext.warrant_list` failed outright whenever the server included a
placeholder row. On `700.HK` with no filters, 324 of the 746 warrants
returned are placeholders — only `symbol`, `name` and `last_done` carry a
value, everything else is an empty string:

```
FilterWarrant { symbol: "68463.HK", name: "HS#TENCTRC2702E",
                last_done: "0.244", expiry_date: "", strike_price: "",
                premium: "", …, status: 0 }
```

The decimal fields already had `.parse().unwrap_or_default()` fallbacks.
Two strict fields did not, and either one aborted the conversion of the
*entire* list:

- `expiry_date: ""` → `parse field: expiry_date: the 'year' component
  could not be parsed`
- `status: 0` → `No discriminant in enum WarrantStatus matches the value
  0` (the enum only had `Suspend = 2` / `PrepareList = 3` / `Normal = 4`)

## Breaking changes

`WarrantInfo.expiry_date` is now optional: Rust `Option<Date>`, Python
`Optional[date]`, Node.js `NaiveDate | null`, Java nullable `LocalDate`,
C `const lb_date_t*` (NULL when absent), C++ `std::optional<Date>`.

`WarrantStatus` gains an `Unknown` variant, placed first to match
`WarrantType`. An unrecognized discriminant now maps to it rather than
failing, so a future unknown status cannot take out the whole list again.
Adding a variant shifts the ordinal of the existing ones in the C, C++,
Java, Node.js and Python bindings.

Placeholder rows now come through as `expiry_date: None, status: Unknown`
instead of being dropped or aborting the call. Verified live from both
Rust and C++: 746 rows returned, 324 of them placeholders.

Also carries the two fixes already submitted separately against `main` as
#579 (null pointers for empty list arguments) and #580 (building and
repairing `asset::AssetContext`) — both were needed to exercise this
change from C++.
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.

2 participants