google / riegeli

Riegeli/records is a file format for storing a sequence of string records, typically serialized protocol buffers.
Apache License 2.0
418 stars 53 forks source link

build error #10

Closed micahcc closed 4 years ago

micahcc commented 4 years ago

Getting the following build error:

With clang-10

clang version 10.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

I'm getting the following error.

0x' -MD -MF bazel-out/k8-fastbuild/bin/python/riegeli/records/_objs/record_position_cc/record_position.pic.d '-frandom-seed=bazel-out/k8-fastbuild/bin/python/riegeli/records/_objs/record_position_cc/record_position.pic.o' -fPIC -D__CLANG_SUPPORT_DYN_ANNOTATION__ -iquote . -iquote bazel-out/k8-fastbuild/bin -iquote external/com_google_absl -iquote bazel-out/k8-fastbuild/bin/external/com_google_absl -iquote external/local_config_python -iquote bazel-out/k8-fastbuild/bin/external/local_config_python -iquote external/highwayhash -iquote bazel-out/k8-fastbuild/bin/external/highwayhash -isystem external/local_config_python/python_include -isystem bazel-out/k8-fastbuild/bin/external/local_config_python/python_include '-std=c++14' '-D_GLIBCXX_USE_CXX11_ABI=0' -no-canonical-prefixes -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -c python/riegeli/records/record_position.cc -o bazel-out/k8-fastbuild/bin/python/riegeli/records/_objs/record_position_cc/record_position.pic.o)
ERROR: /home/ubuntu/repos/riegeli/riegeli/base/BUILD:75:1: C++ compilation of rule '//riegeli/base:buffer' failed (Exit 1) clang failed: error executing command /usr/bin/clang -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer '-std=c++0x' -MD -MF ... (remaining 23 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
In file included from riegeli/base/buffer.cc:15:
In file included from ./riegeli/base/buffer.h:26:
./riegeli/base/memory.h:149:5: error: no matching function for call to 'operator delete'
    operator delete(allocated, sizeof(void*) + num_bytes + alignment -
    ^~~~~~~~~~~~~~~
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/new:144:6: note: candidate function not viable: no known conversion from 'unsigned long' to 'const std::nothrow_t' for 2nd argument
void operator delete(void*, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
     ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/new:180:13: note: candidate function not viable: no known conversion from 'unsigned long' to 'void *' for 2nd argument
inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
            ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/new:130:6: note: candidate function not viable: requires 1 argument, but 2 were provided
void operator delete(void*) _GLIBCXX_USE_NOEXCEPT
     ^
In file included from riegeli/base/buffer.cc:15:
./riegeli/base/buffer.h:100:25: error: no matching function for call to 'operator delete'
  if (data_ != nullptr) operator delete(data_, size_);
                        ^~~~~~~~~~~~~~~
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/new:180:13: note: candidate function not viable: no known conversion from 'size_t' (aka 'unsigned long') to 'void *' for 2nd argument; take the address of the argument with &
inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
            ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/new:144:6: note: candidate function not viable: no known conversion from 'size_t' (aka 'unsigned long') to 'const std::nothrow_t' for 2nd argument
void operator delete(void*, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
     ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/new:130:6: note: candidate function not viable: requires 1 argument, but 2 were provided
void operator delete(void*) _GLIBCXX_USE_NOEXCEPT
     ^
2 errors generated.
Target //python:build_pip_package failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 1.056s, Critical Path: 0.90s
INFO: 14 processes: 14 linux-sandbox.
FAILED: Build did NOT complete successfully

Probably because you specify the --cxxopt=-D_GLIBCXX_USE_CXX11_ABI=0

QrczakMK commented 4 years ago

Interesting. I wonder what would be the proper condition to use sized delete (because it works for me with clang version 9.0.1).

Before https://github.com/google/riegeli/commit/99f5a184a7f77655fef0d5c638cd8dded0ce1edf#diff-b84fe4b731725eecafb89bdc9a8646e7 (which started requiring C++14) sized delete was conditioned on #if __cpp_sized_deallocation || __GXX_DELETE_WITH_SIZE__. Do you think this condition should be restored here?

micahcc commented 4 years ago

So this fixes it:

diff --git a/.bazelrc b/.bazelrc
index 111ddba..2f5e3d6 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -4,6 +4,9 @@ build --cxxopt=-std=c++14
 # Use old C++ ABI, required by TensorFlow dependency.
 build --cxxopt=-D_GLIBCXX_USE_CXX11_ABI=0

+# clang-10 doesn't seem to enable this for c++14, so enable it
+build --cxxopt=-D__cpp_sized_deallocation=1
+
 # Make Python protos faster by backing them with C++ protos.
 build --define=use_fast_cpp_protos=true

But according to this, it should be enabled for -std=c++14: https://en.cppreference.com/w/cpp/feature_test

QrczakMK commented 4 years ago

https://reviews.llvm.org/D8467 suggests that sized deallocation might be disabled in C++14 mode.

https://github.com/cms-sw/cmsdist/pull/5113 suggests that an application might explicitly enable it with -fsized-deallocation, but of course that is unwise to require in a library, especially as this is just an optimization.

I will make using sized deallocation conditional again, with the same guard as before.

QrczakMK commented 4 years ago

Should be fixed by https://github.com/google/riegeli/commit/cf32021e52e840c4a2b8e2fb95e4bfb679c94525.