Skip to content

Commit

Permalink
C++20 compatibility (facebook#6697)
Browse files Browse the repository at this point in the history
Summary:
Based on facebook#6648 (CLA Signed), but heavily modified / extended:

* Implicit capture of this via [=] deprecated in C++20, and [=,this] not standard before C++20 -> now using explicit capture lists
* Implicit copy operator deprecated in gcc 9 -> add explicit '= default' definition
* std::random_shuffle deprecated in C++17 and removed in C++20 -> migrated to a replacement in RocksDB random.h API
* Add the ability to build with different std version though -DCMAKE_CXX_STANDARD=11/14/17/20 on the cmake command line
* Minimal rebuild flag of MSVC is deprecated and is forbidden with /std:c++latest (C++20)
* Added MSVC 2019 C++11 & MSVC 2019 C++20 in AppVeyor
* Added GCC 9 C++11 & GCC9 C++20 in Travis
Pull Request resolved: facebook#6697

Test Plan: make check and CI

Reviewed By: cheng-chang

Differential Revision: D21020318

Pulled By: pdillinger

fbshipit-source-id: 12311be5dbd8675a0e2c817f7ec50fa11c18ab91
  • Loading branch information
pdillinger authored and facebook-github-bot committed Apr 20, 2020
1 parent fe206f4 commit 31da5e3
Show file tree
Hide file tree
Showing 31 changed files with 144 additions and 55 deletions.
47 changes: 46 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,18 @@ env:
- JOB_NAME=examples # 5-7 minutes
- JOB_NAME=cmake # 3-5 minutes
- JOB_NAME=cmake-gcc8 # 3-5 minutes
- JOB_NAME=cmake-gcc9 # 3-5 minutes
- JOB_NAME=cmake-gcc9-c++20 # 3-5 minutes
- JOB_NAME=cmake-mingw # 3 minutes

matrix:
exclude:
- os: osx
env: JOB_NAME=cmake-gcc8
- os: osx
env: JOB_NAME=cmake-gcc9
- os: osx
env: JOB_NAME=cmake-gcc9-c++20
- os: osx
env: JOB_NAME=cmake-mingw
- os: osx
Expand All @@ -70,6 +76,19 @@ matrix:
env: JOB_NAME=cmake-mingw
- os: linux
compiler: clang
# Exclude all but most unique cmake variants for pull requests, but build all in branches
- if: type = pull_request
os : linux
arch: amd64
env: JOB_NAME=cmake
- if: type = pull_request
os : linux
arch: amd64
env: JOB_NAME=cmake-gcc8
- if: type = pull_request
os : linux
arch: amd64
env: JOB_NAME=cmake-gcc9
# Exclude most osx, arm64 and ppc64le tests for pull requests, but build in branches
# Temporarily disable ppc64le unit tests in PRs until Travis gets its act together (#6653)
- if: type = pull_request
Expand Down Expand Up @@ -162,6 +181,22 @@ matrix:
os: linux
arch: ppc64le
env: JOB_NAME=cmake-gcc8
- if: type = pull_request
os : linux
arch: arm64
env: JOB_NAME=cmake-gcc9
- if: type = pull_request
os: linux
arch: ppc64le
env: JOB_NAME=cmake-gcc9
- if: type = pull_request
os : linux
arch: arm64
env: JOB_NAME=cmake-gcc9-c++20
- if: type = pull_request
os: linux
arch: ppc64le
env: JOB_NAME=cmake-gcc9-c++20

install:
- if [ "${TRAVIS_OS_NAME}" == osx ]; then
Expand All @@ -171,6 +206,10 @@ install:
sudo apt-get install -y g++-8;
CC=gcc-8 && CXX=g++-8;
fi
- if [ "${JOB_NAME}" == cmake-gcc9 ] || [ "${JOB_NAME}" == cmake-gcc9-c++20 ]; then
sudo apt-get install -y g++-9;
CC=gcc-9 && CXX=g++-9;
fi
- if [ "${JOB_NAME}" == cmake-mingw ]; then
sudo apt-get install -y mingw-w64 ;
fi
Expand Down Expand Up @@ -240,7 +279,13 @@ script:
mkdir build && cd build && cmake -DJNI=1 -DWITH_GFLAGS=OFF .. -DCMAKE_C_COMPILER=x86_64-w64-mingw32-gcc -DCMAKE_CXX_COMPILER=x86_64-w64-mingw32-g++ -DCMAKE_SYSTEM_NAME=Windows && make -j4 rocksdb rocksdbjni
;;
cmake*)
mkdir build && cd build && cmake -DJNI=1 .. -DCMAKE_BUILD_TYPE=Release && make -j4 rocksdb rocksdbjni
case $JOB_NAME in
*-c++20)
OPT=-DCMAKE_CXX_STANDARD=20
;;
esac

mkdir build && cd build && cmake -DJNI=1 .. -DCMAKE_BUILD_TYPE=Release $OPT && make -j4 rocksdb rocksdbjni
;;
esac
notifications:
Expand Down
21 changes: 16 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ else()
option(WITH_FOLLY_DISTRIBUTED_MUTEX "build with folly::DistributedMutex" OFF)
endif()

if( NOT DEFINED CMAKE_CXX_STANDARD )
set(CMAKE_CXX_STANDARD 11)
endif()

include(CMakeDependentOption)
CMAKE_DEPENDENT_OPTION(WITH_GFLAGS "build with GFlags" ON
"NOT MSVC;NOT MINGW" OFF)
Expand Down Expand Up @@ -191,7 +195,6 @@ else()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-format -fno-asynchronous-unwind-tables")
add_definitions(-D_POSIX_C_SOURCE=1)
endif()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
if(NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")
include(CheckCXXCompilerFlag)
Expand Down Expand Up @@ -254,6 +257,7 @@ include(CheckCXXSourceCompiles)
if(NOT MSVC)
set(CMAKE_REQUIRED_FLAGS "-msse4.2 -mpclmul")
endif()

CHECK_CXX_SOURCE_COMPILES("
#include <cstdint>
#include <nmmintrin.h>
Expand Down Expand Up @@ -386,7 +390,15 @@ if(MSVC)
message(STATUS "Debug optimization is enabled")
set(CMAKE_CXX_FLAGS_DEBUG "/Oxt")
else()
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Od /RTC1 /Gm")
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Od /RTC1")

# Minimal Build is deprecated after MSVC 2015
if( MSVC_VERSION GREATER 1900 )
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Gm-")
else()
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Gm")
endif()

endif()
if(WITH_RUNTIME_DEBUG)
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /${RUNTIME_LIBRARY}d")
Expand Down Expand Up @@ -849,7 +861,6 @@ if(ROCKSDB_BUILD_SHARED)
LINKER_LANGUAGE CXX
VERSION ${rocksdb_VERSION}
SOVERSION ${rocksdb_VERSION_MAJOR}
CXX_STANDARD 11
OUTPUT_NAME "rocksdb")
endif()
endif()
Expand Down Expand Up @@ -1130,7 +1141,7 @@ if(WITH_TESTS)
PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD_RELEASE 1
EXCLUDE_FROM_DEFAULT_BUILD_MINRELEASE 1
EXCLUDE_FROM_DEFAULT_BUILD_RELWITHDEBINFO 1
)
)

foreach(sourcefile ${TESTS})
get_filename_component(exename ${sourcefile} NAME_WE)
Expand All @@ -1140,7 +1151,7 @@ if(WITH_TESTS)
EXCLUDE_FROM_DEFAULT_BUILD_MINRELEASE 1
EXCLUDE_FROM_DEFAULT_BUILD_RELWITHDEBINFO 1
OUTPUT_NAME ${exename}${ARTIFACT_SUFFIX}
)
)
target_link_libraries(${CMAKE_PROJECT_NAME}_${exename}${ARTIFACT_SUFFIX} testutillib${ARTIFACT_SUFFIX} testharness gtest ${ROCKSDB_LIB})
if(NOT "${exename}" MATCHES "db_sanity_test")
add_test(NAME ${exename} COMMAND ${exename}${ARTIFACT_SUFFIX})
Expand Down
17 changes: 14 additions & 3 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
version: 1.0.{build}

image: Visual Studio 2017
image: Visual Studio 2019

environment:
JAVA_HOME: C:\Program Files\Java\jdk1.8.0
Expand All @@ -24,6 +24,15 @@ environment:
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
CMAKE_GENERATOR: Visual Studio 15 Win64
DEV_ENV: C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\devenv.com
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
CMAKE_GENERATOR: Visual Studio 16
CMAKE_PLATEFORM_NAME: x64
DEV_ENV: C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\devenv.com
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
CMAKE_GENERATOR: Visual Studio 16
CMAKE_PLATEFORM_NAME: x64
CMAKE_OPT: -DCMAKE_CXX_STANDARD=20
DEV_ENV: C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\devenv.com

install:
- md %THIRDPARTY_HOME%
Expand All @@ -34,7 +43,8 @@ install:
- cd snappy-1.1.7
- mkdir build
- cd build
- cmake -G "%CMAKE_GENERATOR%" ..
- if DEFINED CMAKE_PLATEFORM_NAME (set "PLATEFORM_OPT=-A %CMAKE_PLATEFORM_NAME%")
- cmake .. -G "%CMAKE_GENERATOR%" %PLATEFORM_OPT%
- msbuild Snappy.sln /p:Configuration=Debug /p:Platform=x64
- msbuild Snappy.sln /p:Configuration=Release /p:Platform=x64
- echo "Building LZ4 dependency..."
Expand All @@ -57,7 +67,8 @@ install:
before_build:
- md %APPVEYOR_BUILD_FOLDER%\build
- cd %APPVEYOR_BUILD_FOLDER%\build
- cmake -G "%CMAKE_GENERATOR%" -DCMAKE_BUILD_TYPE=Debug -DOPTDBG=1 -DPORTABLE=1 -DSNAPPY=1 -DLZ4=1 -DZSTD=1 -DXPRESS=1 -DJNI=1 ..
- if DEFINED CMAKE_PLATEFORM_NAME (set "PLATEFORM_OPT=-A %CMAKE_PLATEFORM_NAME%")
- cmake .. -G "%CMAKE_GENERATOR%" %PLATEFORM_OPT% %CMAKE_OPT% -DCMAKE_BUILD_TYPE=Debug -DOPTDBG=1 -DPORTABLE=1 -DSNAPPY=1 -DLZ4=1 -DZSTD=1 -DXPRESS=1 -DJNI=1
- cd ..

build:
Expand Down
1 change: 1 addition & 0 deletions build_tools/build_detect_platform
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,7 @@ EOF
fi
fi


if [ "$FBCODE_BUILD" != "true" -a "$PLATFORM" = OS_LINUX ]; then
$CXX $COMMON_FLAGS $PLATFORM_SHARED_CFLAGS -x c++ -c - -o test_dl.o 2>/dev/null <<EOF
void dummy_func() {}
Expand Down
7 changes: 7 additions & 0 deletions cmake/modules/CxxFlags.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
macro(get_cxx_std_flags FLAGS_VARIABLE)
if( CMAKE_CXX_STANDARD_REQUIRED )
set(${FLAGS_VARIABLE} ${CMAKE_CXX${CMAKE_CXX_STANDARD}_STANDARD_COMPILE_OPTION})
else()
set(${FLAGS_VARIABLE} ${CMAKE_CXX${CMAKE_CXX_STANDARD}_EXTENSION_COMPILE_OPTION})
endif()
endmacro()
3 changes: 1 addition & 2 deletions db/db_bloom_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1343,8 +1343,7 @@ TEST_F(DBBloomFilterTest, OptimizeFiltersForHits) {
for (int i = 0; i < numkeys; i += 2) {
keys.push_back(i);
}
std::random_shuffle(std::begin(keys), std::end(keys));

RandomShuffle(std::begin(keys), std::end(keys));
int num_inserted = 0;
for (int key : keys) {
ASSERT_OK(Put(1, Key(key), "val"));
Expand Down
2 changes: 1 addition & 1 deletion db/db_compaction_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4762,7 +4762,7 @@ TEST_P(CompactionPriTest, Test) {
for (int i = 0; i < kNKeys; i++) {
keys[i] = i;
}
std::random_shuffle(std::begin(keys), std::end(keys));
RandomShuffle(std::begin(keys), std::end(keys), rnd.Next());

for (int i = 0; i < kNKeys; i++) {
ASSERT_OK(Put(Key(keys[i]), RandomString(&rnd, 102)));
Expand Down
2 changes: 1 addition & 1 deletion db/db_dynamic_level_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ TEST_F(DBTestDynamicLevel, DynamicLevelMaxBytesBase) {
keys[i] = i;
}
if (ordered_insert == 0) {
std::random_shuffle(std::begin(keys), std::end(keys));
RandomShuffle(std::begin(keys), std::end(keys), rnd.Next());
}
for (int max_background_compactions = 1; max_background_compactions < 4;
max_background_compactions += 2) {
Expand Down
6 changes: 3 additions & 3 deletions db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,7 @@ TEST_F(DBTest, ApproximateSizesMemTable) {
keys[i * 3 + 1] = i * 5 + 1;
keys[i * 3 + 2] = i * 5 + 2;
}
std::random_shuffle(std::begin(keys), std::end(keys));
RandomShuffle(std::begin(keys), std::end(keys));

for (int i = 0; i < N * 3; i++) {
ASSERT_OK(Put(Key(keys[i] + 1000), RandomString(&rnd, 1024)));
Expand Down Expand Up @@ -4530,7 +4530,7 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel) {
for (int i = 0; i < kNKeys; i++) {
keys[i] = i;
}
std::random_shuffle(std::begin(keys), std::end(keys));
RandomShuffle(std::begin(keys), std::end(keys));

Random rnd(301);
Options options;
Expand Down Expand Up @@ -4613,7 +4613,7 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel2) {
for (int i = 0; i < kNKeys; i++) {
keys[i] = i;
}
std::random_shuffle(std::begin(keys), std::end(keys));
RandomShuffle(std::begin(keys), std::end(keys));

Random rnd(301);
Options options;
Expand Down
2 changes: 1 addition & 1 deletion db/log_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@

#include <stdio.h>
#include "file/sequence_file_reader.h"
#include "port/lang.h"
#include "rocksdb/env.h"
#include "test_util/sync_point.h"
#include "util/coding.h"
#include "util/crc32c.h"
#include "util/util.h"

namespace ROCKSDB_NAMESPACE {
namespace log {
Expand Down
2 changes: 1 addition & 1 deletion db/memtable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "memory/memory_usage.h"
#include "monitoring/perf_context_imp.h"
#include "monitoring/statistics.h"
#include "port/lang.h"
#include "port/port.h"
#include "rocksdb/comparator.h"
#include "rocksdb/env.h"
Expand All @@ -36,7 +37,6 @@
#include "util/autovector.h"
#include "util/coding.h"
#include "util/mutexlock.h"
#include "util/util.h"

namespace ROCKSDB_NAMESPACE {

Expand Down
4 changes: 2 additions & 2 deletions db/perf_context_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ void ProfileQueries(bool enabled_time = false) {
}

if (FLAGS_random_key) {
std::random_shuffle(keys.begin(), keys.end());
RandomShuffle(std::begin(keys), std::end(keys));
}
#ifndef NDEBUG
ThreadStatusUtil::TEST_SetStateDelay(ThreadStatus::STATE_MUTEX_WAIT, 1U);
Expand Down Expand Up @@ -524,7 +524,7 @@ TEST_F(PerfContextTest, SeekKeyComparison) {
}

if (FLAGS_random_key) {
std::random_shuffle(keys.begin(), keys.end());
RandomShuffle(std::begin(keys), std::end(keys));
}

HistogramImpl hist_put_time;
Expand Down
2 changes: 1 addition & 1 deletion db/write_batch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@
#include "db/write_batch_internal.h"
#include "monitoring/perf_context_imp.h"
#include "monitoring/statistics.h"
#include "port/lang.h"
#include "rocksdb/merge_operator.h"
#include "util/autovector.h"
#include "util/cast_util.h"
#include "util/coding.h"
#include "util/duplicate_detector.h"
#include "util/string_util.h"
#include "util/util.h"

namespace ROCKSDB_NAMESPACE {

Expand Down
2 changes: 2 additions & 0 deletions include/rocksdb/file_system.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ struct FileOptions : EnvOptions {

FileOptions(const FileOptions& opts)
: EnvOptions(opts), io_options(opts.io_options) {}

FileOptions& operator=(const FileOptions& opts) = default;
};

// A structure to pass back some debugging information from the FileSystem
Expand Down
11 changes: 7 additions & 4 deletions memory/concurrent_arena.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <utility>
#include "memory/allocator.h"
#include "memory/arena.h"
#include "port/lang.h"
#include "port/likely.h"
#include "util/core_local.h"
#include "util/mutexlock.h"
Expand Down Expand Up @@ -49,7 +50,7 @@ class ConcurrentArena : public Allocator {

char* Allocate(size_t bytes) override {
return AllocateImpl(bytes, false /*force_arena*/,
[=]() { return arena_.Allocate(bytes); });
[this, bytes]() { return arena_.Allocate(bytes); });
}

char* AllocateAligned(size_t bytes, size_t huge_page_size = 0,
Expand All @@ -58,9 +59,11 @@ class ConcurrentArena : public Allocator {
assert(rounded_up >= bytes && rounded_up < bytes + sizeof(void*) &&
(rounded_up % sizeof(void*)) == 0);

return AllocateImpl(rounded_up, huge_page_size != 0 /*force_arena*/, [=]() {
return arena_.AllocateAligned(rounded_up, huge_page_size, logger);
});
return AllocateImpl(rounded_up, huge_page_size != 0 /*force_arena*/,
[this, rounded_up, huge_page_size, logger]() {
return arena_.AllocateAligned(rounded_up,
huge_page_size, logger);
});
}

size_t ApproximateMemoryUsage() const {
Expand Down
5 changes: 2 additions & 3 deletions memtable/memtablerep_bench.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,8 @@ class KeyGenerator {
for (uint64_t i = 0; i < num_; ++i) {
values_[i] = i;
}
std::shuffle(
values_.begin(), values_.end(),
std::default_random_engine(static_cast<unsigned int>(FLAGS_seed)));
RandomShuffle(values_.begin(), values_.end(),
static_cast<uint32_t>(FLAGS_seed));
}
}

Expand Down
File renamed without changes.
Loading

0 comments on commit 31da5e3

Please sign in to comment.