search

google / benchmark

A microbenchmark support library
Apache License 2.0
8.93k stars 1.61k forks source link

Upgrade google/benchmark to use nanobind-bazel v2.0.0 #1795

Closed nicholasjng closed 2 months ago

nicholasjng commented 4 months ago

This enables binding extension builds with nanobind@v2.

Due to some backwards-incompatible changes in the treatment of enums, Counter::Flags (a flag-based enum) needs to be explicitly marked as arithmetic; setting flags is done as before with the logical "or" operator (now bound like any other class method as __or__(), the Python logical or operator slot).

The bindings changes were suggested by @hawkinsp in the corresponding issue on the repo, see https://github.com/google/benchmark/issues/1790.

Fixes #1790.

nicholasjng commented 4 months ago

Hm, this isn't working yet, I will investigate.

nicholasjng commented 4 months ago

This line in nanobind's enum source:

https://github.com/wjakob/nanobind/blob/4ed5fdf80de460edc416c0e948d6e6c60e61a02a/src/nb_enum.cpp#L46

bases the newly created Flags on the IntEnum class if it's marked as arithmetic, that's presumably where the large negative value comes from in the CI logs.

There is currently a PR open on support for binding Flag enums https://github.com/wjakob/nanobind/pull/599, I could imagine that is necessary to fix this error.

nicholasjng commented 4 months ago

@hawkinsp also please let me know about your preferred attribution for these changes - I expect I'll have to tweak things a bit in the end, but I'd still like to give appropriate credit.

hawkinsp commented 2 months ago

Thanks, the nanobind_bazel change in the PR that was merged was from this PR (and the nanobind_bazel change itself).

nicholasjng commented 2 months ago

yup, I saw. Hoping to release the stubgen rule in nanobind-bazel soon, then we will have IDE completions for benchmarks!