0

For my Masters degree I need to work on a database called duckdb (its on git hub). Normally on Linux you can simply clone it and "make" it to install.

I tired the same on windows after installing CMake and Cygwin.

But halfway through compiling I get the error

'DUCKDB~2/duckdb/THIRD_~1/catch/catch.hpp:1445:96:
error: ISO C++ forbids comparison between pointer and integer [-fpermissive]

 auto compareNotEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return static_cast<bool>(lhs != rhs); }'

Since I doubt that the creators of duckdb did mess this up, I think there is a compiler error trying to compile a C file as C++ file, maybe.

My main problem is: how do I configure the make command on windows to stop it from producing this error?

I tried it both on a windows 7 and 10 system with gcc 5.1 and current cmake installed, and both produce this error.

Edit: Here is the full error text

[ 87%] Building CXX object test/sql/capi/CMakeFiles/test_sql_capi.dir/ub_test_sql_capi.cpp.obj In file included from C:/duckdb/test/sql/capi/test_capi.cpp:1:0, from test_capi.cpp:0:

C:/DUCKDB~2/duckdb/THIRD_~1/catch/catch.hpp: In instantiation of 'bool >Catch::compareNotEqual(const LhsT&, RhsT&&) [with LhsT = void*; RhsT = const >long long int&]':

C:/DB/DUCKDB~2/duckdb/THIRD_~1/catch/catch.hpp:1471:37: required from 'const >Catch::BinaryExpr Catch::ExprLhs::operator!=(const >RhsT&) [with RhsT = long long int; LhsT = void* const&]'

C:/DB/duckdb/test/sql/capi/test_capi.cpp:332:2: required from here C:/DB/DUCKDB~2/duckdb/THIRD_~1/catch/catch.hpp:1445:96: error: ISO C++ forbids comparison >between pointer and integer [-fpermissive] auto compareNotEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return >static_cast(lhs != rhs); }

I only edited out my user name in the path etc.

Hannes Mühleisen
  • 2,542
  • 11
  • 13
Student01
  • 93
  • 7
  • 1
    The error message ( _C++ forbids comparison between pointer and integer_ ) seems pretty clear that the rules have been broken for `C++`. `C` and `C++` are distinct languages, with different rules. _how do I configure the make command on windows to stop it from producing this error?_ Make the offending code compliant with `C++` :) – ryyker Oct 21 '19 at 12:43
  • The offending file seems to be part of a clone of Catch2 and seems to be used only for tests in the library. You should be able to build it without tests. Alternatively, the error message should tell you which of the test `.cpp` file caused the error. (Did you not copy all lines?) Then you could have a look whether something is wrong. The code in question is clearly C++, not C. So that is not the problem. – walnut Oct 21 '19 at 12:55
  • @uneven_mark I don't think it's catch that's at fault. The error message is just incomplete. It looks like a template from the catch library that was instantiated by some other part of the project – PeterT Oct 21 '19 at 12:57
  • @PeterT I didn't mean to say that Catch2 is at fault, just that it is where the error manifested. – walnut Oct 21 '19 at 12:58
  • In CMake you can do `include_directories(SYSTEM some_library)` so it ignores warnings from those headers. – nada Oct 21 '19 at 13:04
  • 1
    @nada It is an error, not a warning. And I would not add `-fpermissive` to the flags. – walnut Oct 21 '19 at 13:06
  • @uneven_mark If OP has no jurisdiction over the contents of some library the only options are to 1) either ignore problems during its compilation or 2) take it to its devs. – nada Oct 21 '19 at 13:11
  • @nada Yes sure, ultimately this is the correct approach in any case. I just wanted to note that `include_directories(SYSTEM some_library)` won't silence this error. – walnut Oct 21 '19 at 13:12
  • @uneven_mark Why not? I assumed it's in an included header. – nada Oct 21 '19 at 13:14
  • @nada I just tested it with GCC 9.2.0, `-isystem` does not silence the pointer/integer comparison error without adding the `-fpermissive` flag. – walnut Oct 21 '19 at 13:17
  • It works in my environment as long as I define my `CMAKE_CXX_FLAGS` _after_ doing `include_directories(SYSTEM some_library)` (which also has warnings in my case but ¯\\_(ツ)\_/¯ ) – nada Oct 21 '19 at 13:20

3 Answers3

2

I don't know the library, so I can't give definite answer. I will be going by the code at https://github.com/cwida/duckdb.

According to the error message in the problematic code is in line 332 of test/sql/capi/test_capi.cpp, which is:

REQUIRE(stmt != NULL);

REQUIRE is a macro from the unit testing library Catch2 that does some magic to parse the expression given to it. The important part is that stmt != NULL will not actually be executed immediately, but only through function indirection.

stmt is declared as in line 324 as

duckdb_prepared_statement stmt = nullptr;

and duckdb_prepared_statement is a typedef in line 94 of src/include/duckdb.h:

typedef void *duckdb_prepared_statement;

Therefore the intention of the problematic line is to check whether stmt is still a nullptr after some intermediate operations.

Normally stmt != NULL would be fine for that. However because the Catch2 macro introduces intermediate function calls rather than evaluating this expression directly, implicit conversions that are specific to the literal are not applied.

In particular NULL is according to the standard either a prvalue of type std::nullptr_t or an integer literal with value 0. Which of these exactly (and which integer type) is implementation-defined.

Comparing integers and pointers is generally forbidden, however integer literals of value zero have a special exception allowing them to be implicitly converted to a null pointer of any pointer type. This is what would happen if stmt != NULL would be evaluated directly.

However due to the interjection of Catch2, NULL is first passed around and later compared through a variable to stmt, which makes the special zero literal rule not apply any more. Therefore a comparison of stmt against NULL through REQUIRE would fail if NULL is an integer literal in the current implementation.

Catch2 does consider this problem and there are overloads for compareNotEqual in third_party/catch/catch.hpp that take care of the case where NULL is a zero integer literal of type int or long, but for some reason the case of long long is not considered. I don't know whether this is a problem with Catch2 or whether it is only in the cloned version included in duckdb.

So, if the implementation uses a zero literal of type long long for NULL, then the error you observed will happen.

Really duckdb should use nullptr instead of NULL (as it does in the initialization), which does not have these problems and was added to the language because of exactly these problems.

I suppose you can simply try to fix this issue by replacing NULL with nullptr (maybe in other test cases as well).

However the problematic code is only in files which are themselves unit tests for the actual library code. So there should be some option to cmake or make that will disable building the unit tests, so that you can ignore this particular error, hoping that it doesn't occur anywhere in the actual library code as well.

If I was correct in my assessment, you might want consider filing a bug report for this with duckdb, assuming they do support your platform in the first place.

walnut
  • 21,629
  • 4
  • 23
  • 59
1

Rather than using Cygwin, it might be easier to use CMake combined with Visual Studio to compile the project. This is the way that the authors compile and test DuckDB on Windows themselves.

Using CMake, generate a set of Visual Studio Project files. Then open those project files in Visual Studio (we use Visual Studio Community 2019), and compile and run the test suite (unittest). Individual tests can be run by adding the test name as a command line parameter.

Mytherin
  • 131
  • 3
0

DuckDB doesn't seem to have a release version at this time. So don't assume it the creators didn't leave any errors in...

You can add compiler flag -fpermissive to ignore the error (e.g. make CXX="g++ -fpermissive").

But ignoring errors like this isn't always safe.

So I tried to fix the issue, and I was able to build a version on Windows. See https://github.com/cwida/duckdb/issues/361 for my solution.

Brecht Sanders
  • 6,215
  • 1
  • 16
  • 40
  • I noticed your answer and the link to the issue. Just letting you know that your diff posted there will break the function for other input types (it is supposed to work with all types not only types convertible to `uintptr_t`). The correct fix is to use `nullptr` instead of `NULL` as I mentioned in my answer or to add an additional overload `template auto compareNotEqual( T* const& lhs, long long rhs ) -> bool { return lhs != reinterpret_cast( rhs ); }` similar to the ones that already exist (just `long long` instead of `long`) and analogues for `compareEqual`. – walnut Nov 25 '19 at 12:25
  • If I was wrong and that doesn't fix the issue I would appreciate if you let me know so that I can improve my answer. I have not tested it myself. – walnut Nov 25 '19 at 12:42
  • The patch at https://github.com/cwida/duckdb/commit/a565715deb9351ecc39b8d04038f21e8cb8fd836 gives me this result: `R:/x86_64-7.2.0-release-posix-seh-rt_v6-rev0/duckdb-0.1.1/src/common/types/value.cpp:481:10: error: converting to 'bool' from 'std::nullptr_t' requires direct-initialization [-fpermissive]` – Brecht Sanders Nov 26 '19 at 19:40
  • That seems to be related to the `NULL` -> `nullptr` replacement in that file. Are you sure you applied the patch correctly? Line 481 is the closing brace and `nullptr` is only compared to `s`, `std::nullptr_t` is not used anywhere else, it is nowhere (attempted) to convert it to `bool`. (Again I have not tested myself.) See https://github.com/cwida/duckdb/blob/a565715deb9351ecc39b8d04038f21e8cb8fd836/src/common/types/value.cpp#L481 – walnut Nov 26 '19 at 20:36