Skip to content

Conversation

@alexkaratarakis
Copy link
Contributor

alexkaratarakis added a commit to alexkaratarakis/fixed-containers that referenced this pull request Jun 14, 2024
alexkaratarakis added a commit to alexkaratarakis/fixed-containers that referenced this pull request Jun 14, 2024
google/benchmark#1803

```
In file included from test/fixed_map_perf_test.cpp:4:
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:1801:25: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
 1801 |           memory_result(NULL),
      |                         ^~~~
      |                         nullptr
/usr/lib/llvm-17/lib/clang/17/include/stddef.h:84:18: note: expanded from macro 'NULL'
   84 | #    define NULL __null
      |                  ^
In file included from test/fixed_map_perf_test.cpp:4:
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:378:7: error: 'MemoryManager' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
  378 | class MemoryManager {
      |       ^
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:1437:7: error: 'Fixture' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
 1437 | class Fixture : public internal::Benchmark {
      |       ^
3 errors generated.

```
alexkaratarakis added a commit to teslamotors/fixed-containers that referenced this pull request Jun 14, 2024
google/benchmark#1803

```
In file included from test/fixed_map_perf_test.cpp:4:
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:1801:25: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
 1801 |           memory_result(NULL),
      |                         ^~~~
      |                         nullptr
/usr/lib/llvm-17/lib/clang/17/include/stddef.h:84:18: note: expanded from macro 'NULL'
   84 | #    define NULL __null
      |                  ^
In file included from test/fixed_map_perf_test.cpp:4:
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:378:7: error: 'MemoryManager' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
  378 | class MemoryManager {
      |       ^
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:1437:7: error: 'Fixture' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
 1437 | class Fixture : public internal::Benchmark {
      |       ^
3 errors generated.

```
@dmah42
Copy link
Member

dmah42 commented Jun 14, 2024

i had originally decided to not use includes for exactly this reason: that it is not a system include and would normally be included with a manual installation as -I rather than -isystem. however, i recognise the diagnostic issues that can be exposed through this, and that this PR simplifies integration for clients, so i'm happy to accept the change.

thanks so much :)

@dmah42
Copy link
Member

dmah42 commented Jun 14, 2024

looks like precommit has an opinion about ordering of fields.

@alexkaratarakis alexkaratarakis force-pushed the benchmark_as_system_include branch from a53f931 to 4af61d1 Compare June 15, 2024 08:28
@alexkaratarakis
Copy link
Contributor Author

looks like precommit has an opinion about ordering of fields.

Fixed and verified locally:

$ pre-commit run --all-files
buildifier...............................................................Passed
buildifier-lint..........................................................Passed
mypy.....................................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed

@dmah42
Copy link
Member

dmah42 commented Jun 17, 2024

unrelated failure. thank you for the fix.

@dmah42 dmah42 merged commit 4477525 into google:main Jun 17, 2024
@alexkaratarakis alexkaratarakis deleted the benchmark_as_system_include branch June 18, 2024 06:59
alexkaratarakis added a commit to alexkaratarakis/fixed-containers that referenced this pull request Jun 18, 2024
alexkaratarakis added a commit to teslamotors/fixed-containers that referenced this pull request Jun 18, 2024
EazyReal pushed a commit to EazyReal/fixed-containers that referenced this pull request Jul 16, 2024
google/benchmark#1803

```
In file included from test/fixed_map_perf_test.cpp:4:
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:1801:25: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
 1801 |           memory_result(NULL),
      |                         ^~~~
      |                         nullptr
/usr/lib/llvm-17/lib/clang/17/include/stddef.h:84:18: note: expanded from macro 'NULL'
   84 | #    define NULL __null
      |                  ^
In file included from test/fixed_map_perf_test.cpp:4:
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:378:7: error: 'MemoryManager' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
  378 | class MemoryManager {
      |       ^
bazel-out/k8-fastbuild/bin/external/com_google_benchmark/_virtual_includes/benchmark_main/benchmark/benchmark.h:1437:7: error: 'Fixture' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
 1437 | class Fixture : public internal::Benchmark {
      |       ^
3 errors generated.

```
EazyReal pushed a commit to EazyReal/fixed-containers that referenced this pull request Jul 16, 2024
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