Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fail with gcc 9 #5303

Open
BusyJay opened this issue May 14, 2019 · 5 comments

Comments

Projects
None yet
4 participants
@BusyJay
Copy link
Contributor

commented May 14, 2019

Expected behavior

Compilation completes successfully.

Actual behavior

Compilation failure:

$DEBUG_LEVEL is 1
Makefile:168: Warning: Compiling in debug mode. Don't use the resulting binary in production
  GEN      util/build_version.cc
$DEBUG_LEVEL is 1
Makefile:168: Warning: Compiling in debug mode. Don't use the resulting binary in production
  GEN      util/build_version.cc
  CC       cache/clock_cache.o
  CC       cache/lru_cache.o
  CC       cache/sharded_cache.o
  CC       db/builder.o
In file included from ./db/memtable.h:21,
                 from ./db/memtable_list.h:17,
                 from ./db/column_family.h:17,
                 from ./db/version_set.h:31,
                 from ./db/compaction.h:11,
                 from ./db/compaction_iterator.h:13,
                 from db/builder.cc:16:
./db/version_edit.h: In constructor ‘rocksdb::FdWithKeyRange::FdWithKeyRange(rocksdb::FileDescriptor, rocksdb::Slice, rocksdb::Slice, rocksdb::FileMetaData*)’:
./db/version_edit.h:178:33: error: implicitly-declared ‘constexpr rocksdb::FileDescriptor::FileDescriptor(const rocksdb::FileDescriptor&)’ is deprecated [-Werror=deprecated-copy]
  178 |         largest_key(_largest_key) {}
      |                                 ^
./db/version_edit.h:55:19: note: because ‘rocksdb::FileDescriptor’ has user-provided ‘rocksdb::FileDescriptor& rocksdb::FileDescriptor::operator=(const rocksdb::FileDescriptor&)’
   55 |   FileDescriptor& operator=(const FileDescriptor& fd) {
      |                   ^~~~~~~~
./db/version_edit.h: In instantiation of ‘constexpr std::pair<_T1, _T2>::pair(_U1&&, _U2&&) [with _U1 = int&; _U2 = rocksdb::FileMetaData; typename std::enable_if<(std::_PCC<true, _T1, _T2>::_MoveConstructiblePair<_U1, _U2>() && std::_PCC<true, _T1, _T2>::_ImplicitlyMoveConvertiblePair<_U1, _U2>()), bool>::type <anonymous> = true; _T1 = int; _T2 = rocksdb::FileMetaData]’:
/usr/include/c++/9/ext/new_allocator.h:147:4:   required from ‘void __gnu_cxx::new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = std::pair<int, rocksdb::FileMetaData>; _Args = {int&, rocksdb::FileMetaData}; _Tp = std::pair<int, rocksdb::FileMetaData>]’
/usr/include/c++/9/bits/alloc_traits.h:484:4:   required from ‘static void std::allocator_traits<std::allocator<_CharT> >::construct(std::allocator_traits<std::allocator<_CharT> >::allocator_type&, _Up*, _Args&& ...) [with _Up = std::pair<int, rocksdb::FileMetaData>; _Args = {int&, rocksdb::FileMetaData}; _Tp = std::pair<int, rocksdb::FileMetaData>; std::allocator_traits<std::allocator<_CharT> >::allocator_type = std::allocator<std::pair<int, rocksdb::FileMetaData> >]’
/usr/include/c++/9/bits/vector.tcc:115:30:   required from ‘void std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {int&, rocksdb::FileMetaData}; _Tp = std::pair<int, rocksdb::FileMetaData>; _Alloc = std::allocator<std::pair<int, rocksdb::FileMetaData> >]’
./db/version_edit.h:253:48:   required from here
./db/version_edit.h:86:8: error: implicitly-declared ‘constexpr rocksdb::FileDescriptor::FileDescriptor(const rocksdb::FileDescriptor&)’ is deprecated [-Werror=deprecated-copy]
   86 | struct FileMetaData {
      |        ^~~~~~~~~~~~
./db/version_edit.h:55:19: note: because ‘rocksdb::FileDescriptor’ has user-provided ‘rocksdb::FileDescriptor& rocksdb::FileDescriptor::operator=(const rocksdb::FileDescriptor&)’
   55 |   FileDescriptor& operator=(const FileDescriptor& fd) {
      |                   ^~~~~~~~
In file included from /usr/include/c++/9/bits/stl_algobase.h:64,
                 from /usr/include/c++/9/bits/char_traits.h:39,
                 from /usr/include/c++/9/string:40,
                 from ./db/builder.h:9,
                 from db/builder.cc:10:
/usr/include/c++/9/bits/stl_pair.h:342:64: note: synthesized method ‘rocksdb::FileMetaData::FileMetaData(rocksdb::FileMetaData&&)’ first required here
  342 |  : first(std::forward<_U1>(__x)), second(std::forward<_U2>(__y)) { }
      |                                                                ^
In file included from ./db/memtable.h:21,
                 from ./db/memtable_list.h:17,
                 from ./db/column_family.h:17,
                 from ./db/version_set.h:31,
                 from ./db/compaction.h:11,
                 from ./db/compaction_iterator.h:13,
                 from db/builder.cc:16:
./db/version_edit.h: In instantiation of ‘constexpr std::pair<_T1, _T2>::pair(_U1&&, const _T2&) [with _U1 = int&; typename std::enable_if<std::_PCC<true, _T1, _T2>::_MoveCopyPair<true, _U1, _T2>(), bool>::type <anonymous> = true; _T1 = int; _T2 = rocksdb::FileMetaData]’:
/usr/include/c++/9/ext/new_allocator.h:147:4:   required from ‘void __gnu_cxx::new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = std::pair<int, rocksdb::FileMetaData>; _Args = {int&, const rocksdb::FileMetaData&}; _Tp = std::pair<int, rocksdb::FileMetaData>]’
/usr/include/c++/9/bits/alloc_traits.h:484:4:   required from ‘static void std::allocator_traits<std::allocator<_CharT> >::construct(std::allocator_traits<std::allocator<_CharT> >::allocator_type&, _Up*, _Args&& ...) [with _Up = std::pair<int, rocksdb::FileMetaData>; _Args = {int&, const rocksdb::FileMetaData&}; _Tp = std::pair<int, rocksdb::FileMetaData>; std::allocator_traits<std::allocator<_CharT> >::allocator_type = std::allocator<std::pair<int, rocksdb::FileMetaData> >]’
/usr/include/c++/9/bits/vector.tcc:115:30:   required from ‘void std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {int&, const rocksdb::FileMetaData&}; _Tp = std::pair<int, rocksdb::FileMetaData>; _Alloc = std::allocator<std::pair<int, rocksdb::FileMetaData> >]’
./db/version_edit.h:258:37:   required from here
./db/version_edit.h:86:8: error: implicitly-declared ‘constexpr rocksdb::FileDescriptor::FileDescriptor(const rocksdb::FileDescriptor&)’ is deprecated [-Werror=deprecated-copy]
   86 | struct FileMetaData {
      |        ^~~~~~~~~~~~
./db/version_edit.h:55:19: note: because ‘rocksdb::FileDescriptor’ has user-provided ‘rocksdb::FileDescriptor& rocksdb::FileDescriptor::operator=(const rocksdb::FileDescriptor&)’
   55 |   FileDescriptor& operator=(const FileDescriptor& fd) {
      |                   ^~~~~~~~
In file included from /usr/include/c++/9/bits/stl_algobase.h:64,
                 from /usr/include/c++/9/bits/char_traits.h:39,
                 from /usr/include/c++/9/string:40,
                 from ./db/builder.h:9,
                 from db/builder.cc:10:
/usr/include/c++/9/bits/stl_pair.h:312:51: note: synthesized method ‘rocksdb::FileMetaData::FileMetaData(const rocksdb::FileMetaData&)’ first required here
  312 |        : first(std::forward<_U1>(__x)), second(__y) { }
      |                                                   ^
cc1plus: all warnings being treated as errors
make: *** [Makefile:1953: db/builder.o] Error 1

Steps to reproduce the behavior

% gcc --version                                                                                                                               :(
gcc (GCC) 9.1.1 20190503 (Red Hat 9.1.1-1)
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
% uname -r
5.0.13-300.fc30.x86_64

Simple make command can reproduce the error.

@wanghenshui

This comment has been minimized.

Copy link

commented May 14, 2019

Introduced by gcc 9's new warning checker [-Werror=deprecated-copy]

Here is a fix example:
tsduck/tsduck#205

@BusyJay

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

It doesn't compile with clang 8 either unfortunately.

In file included from db/builder.cc:19:
./db/internal_stats.h:112:5: error: declaration shadows a variable in namespace 'rocksdb' [-Werror,-Wshadow]
    WAL_FILE_BYTES,
    ^
./include/rocksdb/statistics.h:196:3: note: previous declaration is here
  WAL_FILE_BYTES,   // Number of bytes written to WAL
  ^
In file included from db/builder.cc:19:
./db/internal_stats.h:113:5: error: declaration shadows a variable in namespace 'rocksdb' [-Werror,-Wshadow]
    WAL_FILE_SYNCED,
    ^
@xenog

This comment has been minimized.

Copy link

commented May 28, 2019

I managed to build and install RocksDB on my Fedora 30 system with these commands:

CXXFLAGS='-Wno-error=deprecated-copy -Wno-error=pessimizing-move' make shared_lib
sudo make install-shared

I hope that information is useful for anyone else trying.

@biocodz

This comment has been minimized.

Copy link

commented Jun 6, 2019

The same problem using GCC 9.1 on Ubuntu 18.04 to build rocksdb/master.

The deprecated-copy is thrown because:
Here a copy assignment operator is defined.
Here FileDescriptor fd; is declared.
Each time a copy of FileDescriptor needs to be created e.g.
@175: : fd(_fd), the FileDescriptor copy-constructor is invoked
or
@258: new_files_.emplace_back(level, f); (copy-constructor is automatically called when pushing to vector),
the warning is thrown.
According to C++ docs : The generation of the implicitly-defined copy constructor is deprecated if T has a user-defined destructor or user-defined copy assignment operator. Since C++11
Apparently GCC 8,9 enforce this deprecation.

Explicit copy constructor needs to be implemented, which is also a good practice alongside copy-assignment as follows:

  FileDescriptor(const FileDescriptor& fd) {
    table_reader = fd.table_reader;
    packed_number_and_path_id = fd.packed_number_and_path_id;
    file_size = fd.file_size;
    smallest_seqno = fd.smallest_seqno;
    largest_seqno = fd.largest_seqno;
  }

The pessimizing-move warning is caused by this line.
The call std::move(t) has to be removed i.e. simply return t to allow for the copy elision i.e. to avoid generation of a sub-optimal code.

Using CXXFLAGS is of course only a temporary solution.

After fixing the above errors as described (and the fix works fine), getting the following:

/home/biocodz/rocksdb/db/internal_stats.cc: In member function ‘void rocksdb::InternalStats::DumpCFStatsNoFileHistogram(std::string*)’:
/home/biocodz/rocksdb/db/internal_stats.cc:1384:35: error: implicitly-declared ‘rocksdb::InternalStats::CompactionStats& rocksdb::InternalStats::CompactionStats::operator=(const rocksdb::InternalStats::CompactionStats&)’ is deprecated [-Werror=deprecated-copy]
		 1384 |   cf_stats_snapshot_.comp_stats = compaction_stats_sum;
			  |                                   ^~~~~~~~~~~~~~~~~~~~
In file included from /home/biocodz/rocksdb/db/internal_stats.cc:11:
/home/biocodz/rocksdb/db/internal_stats.h:220:14: note: because ‘rocksdb::InternalStats::CompactionStats’ has user-provided ‘rocksdb::InternalStats::CompactionStats::CompactionStats(const rocksdb::InternalStats::CompactionStats&)’
		  220 |     explicit CompactionStats(const CompactionStats& c)
			  |              ^~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
CMakeFiles/rocksdb-shared.dir/build.make:530: recipe for target 'CMakeFiles/rocksdb-shared.dir/db/internal_stats.cc.o' failed

So, now the copy constructor is defined but not the copy assignment => implement.

@biocodz

This comment has been minimized.

Copy link

commented Jun 7, 2019

Voila, a pull request here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.