Initial partial CMake support for abseil, created after the discussion of this afternoon.

Do not merge yet, this is still incomplete.

I created the pull request for tracking and avoid duplicated effort.

Adev

Issue following the discussions on #8 : should we support install() in the cmake build:

Pro:

• required by people who do not use cmake in their project.
• make the usage of package manager possible

Cons:

• make easy for to rely on ABI and have problems

Hello,

I can't see any version number / scheme in the library. Does it exist? It would be nice to have such version numbers reflected via git tags (and probably the cmake file) in order to:

• clone the repository on a specific tag in order to ensure repeatable builds & environment setup
• know if a new version has been released and compare what's new regarding the version we are currently using

That would probably imply to maintain a changelog file as well. It's a bit of more work but it's very useful for consumers like me.

Aurelien

Currently it appears to be impossible to compile Abseil using VS 2017 without errors.

Adding something like this resolve this problem with windows.h includes.

# ifndef WIN32_LEAN_AND_MEAN
#   define WIN32_LEAN_AND_MEAN
# endif
# ifndef VC_EXTRALEAN
#   define VC_EXTRALEAN
# endif
# ifndef NOMINMAX
#   define NOMINMAX
# endif
#include <windows.h>

see https://greg7mdp.github.io/parallel-hashmap/ for writeup.

The new absl::parallel_flat_hash_map and absl::parallel_flat_hash_set have the same tests as the base maps and all tests pass on Windows (vs2017) and linux (g++ 5.4 and clang 3.8)

No matter what I try, cmake builds the whole abseil and then links everything into my shared library. I could reproduce this with a minimal cmake example:

cmake_minimum_required(VERSION 2.8.12)
project(my_project)
set(CMAKE_CXX_FLAGS "-std=c++11 -fvisibility=hidden ${CMAKE_CXX_FLAGS}")
add_subdirectory(abseil-cpp)
add_library(my_lib SHARED main.cpp)
include_directories(SYSTEM abseil-cpp)
target_link_libraries(my_lib absl::hash absl::container)

main.cpp contains only the following:

#include <absl/container/flat_hash_map.h>
int foo(void) {
	absl::flat_hash_map<int,int> m;
	return 0;
}

For an actual project where I'm trying to use abseil, I get a linker error in debug mode that says the following:

/usr/sbin/ld: ../absail/absl/container/libabsl_container.a(raw_hash_set.cc.o): relocation R_X86_64_TPOFF32 against hidden symbol `_ZZN4absl18container_internal10RandomSeedEvE7counter' can not be used when making a shared object

The project in question is here. Top level CmakeLists.txt Shared object defining CMakeLists.txt Abseil, at commit a06c4a1, is a submodule of my project. The build instructions are:

• Clone bstaletic/ycmd and checkout aseil branch
• git submodule update --init --recursive
• mkdir build && cd build
• cmake ../cpp -DCMAKE_BUILD_TYPE=Debug

Other build time dependencies are python headers. For python3 -DUSE_PYTHON2=OFF is needed.

This is not a bug report, more of an interesting data point. In the past week I've been trying out absl::flat_hash_map and google::dense_hash_map (and sets) and comparing performance to STL counterparts.

The following gist contains benchmarks for STL (Master), abseil and dense_hash_map: https://gist.github.com/bstaletic/7d3e31db7fc8d40a12787fab09ff092f

The project whose benchmark results you're looking at is https://github.com/Valloric/ycmd The benchmarks test two separate algorithms, FilterAndSortCandidates and IdentifierCompleter.

FilterAndSortCandidates benchmark

This is benchmarking the function of the same name. This one actually has the best timings with abseil.

IdentifierCompleter benchmark

This is benchmarking this piece of code. Surprisingly, abseil is the slowest of the three.

If there's interest to investigate my benchmark results further, I will gladly provide any information needed.

Hello,

I have a bit of trouble using a local Abseil installation, as I dont want to depend on external packages with some fixes URL nor some fixed filesystem paths. Cmake already HAS the necessary infrastructure to allow building dependend modules.

For using Abseil in your Project, just this would be necessary:

# cctz pulls in some gtest and benchmark suites otherwise
set (BUILD_TESTING OFF)
find_package(Abseil REQUIRED
)

And, given the worst-case that abseil and cctz are in no directory that can be found automatically, configuration would add the respective paths: cmake -DAbseil_DIR=<local path> -DCctz_DIR=<local path> ...

Basic support would be added by this patch (note that cctz would need to be fixed in a similar fashion):

diff --git a/AbseilConfig.cmake b/AbseilConfig.cmake
new file mode 100644
index 0000000..4476b98
--- /dev/null
+++ b/AbseilConfig.cmake
@@ -0,0 +1,2 @@
+# Macros
+include(${CMAKE_CURRENT_LIST_DIR}/CMake/AbseilMacros.cmake)
diff --git a/CMake/AbseilMacros.cmake b/CMake/AbseilMacros.cmake
new file mode 100644
index 0000000..94c6886
--- /dev/null
+++ b/CMake/AbseilMacros.cmake
@@ -0,0 +1,8 @@
+# TODO: this should be another find_package(Cctz)
+add_subdirectory(${Cctz_DIR} _libcctz
+	EXCLUDE_FROM_ALL
+)
+
+add_subdirectory("${CMAKE_CURRENT_LIST_DIR}/.." _libabseil
+	EXCLUDE_FROM_ALL
+)

Randen expects seed and state buffers to be in LE ordering even on BE machines, and PCG is not, but they both use a shared seed implementation which is currently favouring Randen.

This patch fixes randen_slow to behave the same as RandenHwAes and reverse the byte order when necessary and changes seed generator to be in native byte order.

Defining an empty default constructor is worse than using the compiler generated constructor.

Note that the compiler generated constructor requires that constexpr as well as exception specifiers are correctly considered. See Chapter 15.1 [class.ctor] paragraph 7

Request for a function like string_view StrReadUntil(string_view&, string_view sep)

Returns the part until (not including) the separator and removes that part (including separator) from the input (in-place). Returns the entire input and clears it if separator isn't found.

Examples: StrReadUntil(is, "\n"); // reads next line StrReadUntil(is, ";"); reads next value from 1;2;3

It's a bit like getline() for string_views.

off64_t is not provided without defining _LARGEFILE64_SOURCE on musl this define is not defined automatically like glibc where it gets defined when _GNU_SOURCE is defined. Using off_t makes it portable across musl/glibc and for using 64bit off_t on glibc 32bit systems -D_FILE_OFFSET_BITS=64 can be defined during build via CXXFLAGS

Signed-off-by: Khem Raj [email protected]

As known, (see https://github.com/abseil/abseil-cpp/blob/master/absl/base/internal/atomic_hook.h#L139, https://github.com/abseil/abseil-cpp/issues/659, etc.), MSVC does not provide a constinit std::atomic<T*>.

But, both https://github.com/abseil/abseil-cpp/blob/master/absl/strings/internal/cordz_handle.cc#L27 and https://github.com/abseil/abseil-cpp/blob/master/absl/strings/internal/cordz_info.cc#L41 attempt to define a ABSL_CONST_INIT object which contain a std::atomic (https://github.com/abseil/abseil-cpp/blob/master/absl/strings/internal/cordz_handle.h#L90, https://github.com/abseil/abseil-cpp/blob/master/absl/strings/internal/cordz_info.h#L196).

This makes absl not compilable with the latest MSVC.

I see a macro ABSL_INTERNAL_CORDZ_ENABLED https://github.com/abseil/abseil-cpp/blob/72ec15a317a74cccf03a62f749f3ab28206be069/absl/strings/internal/cordz_functions.h#L47, which is false for MSVC; does this mean cordz_info and cordz_handle are not even needed, as a simple 'fix'? Or do we need a different workaround?

absl\strings\internal\cordz_info.cc:41:44: error: variable does not have a constant initializer
ABSL_CONST_INIT CordzInfo::List CordzInfo::global_list_{absl::kConstInit};
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
absl\strings\internal\cordz_info.cc:41:1: note: required by 'require_constant_initialization' attribute here
ABSL_CONST_INIT CordzInfo::List CordzInfo::global_list_{absl::kConstInit};
^~~~~~~~~~~~~~~
absl/base/attributes.h:714:27: note: expanded from macro 'ABSL_CONST_INIT'
#define ABSL_CONST_INIT [[clang::require_constant_initialization]]
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...xxatomic:207:21: note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression
                : _Atomic_address{(_ATOMIC_UINT)_Right}
                                  ^
abseil-cpp\absl/strings/internal/cordz_info.h:196:56: note: in call to 'atomic(nullptr)'
    std::atomic<CordzInfo*> head ABSL_GUARDED_BY(mutex){nullptr};

absl's MakeSpan takes only pointers, but since vector is very commonly used in C++, I've come across many use-cases where people tried to make a span for a vector in a similar way as array, and the resultant code was actually a UB. The example in the comments is for arrays:

int array[3] = { 0, 0, 0 };
// Call with a [begin, end) pair.
ProcessInts(absl::MakeConstSpan(&array[0], &array[3]));

I've seen many people do the same for vectors, and access the element at index size(), which is UB. A better way to do that might be to use data() method of vectors, like MakeConstSpan(v.data(), v.data() + 3).

Can we have a comment saying this, or maybe we can add a method with iterators. I think even in the example in the comments, &array[3] might as well be UB, since I'm not sure about accessing the element at the size() index for an array.

Can you please share your thoughts on this?

https://github.com/abseil/abseil-cpp/blob/master/absl/crc/internal/crc_internal.h#L171

I cannot find in source who use this function (not used in source or tests or benchmarks). But because of it vector and memory included.

Maybe remove it and unnecessary includes?

https://github.com/abseil/abseil-cpp/blob/842560d214649fc0077838e5b02cc35e4af12526/absl/crc/crc32c.h#L169

Firstly, header includes both <iostream> and <ostream>, which seems redundantly. And operator << defined for std::ostream, but not for std::basic_ostream<Char, Traits>. It breakes code like std::wcout << value;

I dont think output operator is needed generally, atleast in this header

Also personally i dont see a reason to not make crc32c a type(class) without free functions