Building a library that depends on clingo with `DCLASP_BUILD_WITH_THREADS=Off` about clingo HOT 10 CLOSED

@nuernbergk this is to let you know that @BenKaufmann added a second patch that addresses the problem at the root. With this it should be possible to build clasp using C++98 and still use it in code requiring later standards.

from clingo.

Enabling threads also enables C++11 in clasp and static_assert comes along with it. It seems like there is some cmake configuration that is wrong. I would recommend to build without threads and enable C++14 or higher project wide.

from clingo.

Wouldn't that mean I should get this issue only when building with threads? It seems odd that it happens when I try to build without threads.
I tried to set C++14 project wide, but it doesn't help. In any case, this shouldn't be the problem, since the clingo CMakeLists.txt requires the C++14 standard as well.

from clingo.

This message shows that the static_assert of libpotassco is used somewhere within the STL:

min_element.h:57:61: error: too many arguments provided to function-like macro invocation

It should not happen if everything is compiled with C++14. I cannot really tell you more. I would compile in verbose mode and have a look at the compiler commands to check if compiler options are used consistently.

from clingo.

Thank you so much. Indeed, the problem is in the CMakeLists.txt for Clasp, lines L98-L100, where the standard is set to 98. Simply changing this to be 11 works.

Is this desired behaviour? It seems like it would make more sense to not set the standard at all in the case that threads aren't used. For my use case, it is of course fine to simply patch this file, but this does seem like an odd design choice.

from clingo.

Thank you so much. Indeed, the problem is in the CMakeLists.txt for Clasp, lines L98-L100, where the standard is set to 98. Simply changing this to be 11 works.

Is this desired behaviour? It seems like it would make more sense to not set the standard at all in the case that threads aren't used. For my use case, it is of course fine to simply patch this file, but this does seem like an odd design choice.

Clasp is written in C++98. Setting the standard to C++98 is the right thing to do.

@BenKaufmann I think there is a problem how libc++ uses static_assert. Maybe we should patch this in libpotassco by simply using another name? We could also add an option to not set the standard at all and use whatever is the default or configured by projects like clingo using clasp.

from clingo.

I think there is a problem how libc++ uses static_assert. Maybe we should patch this in libpotassco by simply using another name? We could also add an option to not set the standard at all and use whatever is the default or configured by projects like clingo using clasp.

@rkaminsk I'd be fine with either option. However, I'd first like to understand the underlying problem better - i.e. why is the detection code in libpotassco's platform.h not working and/or written like it is?

• The error message implies that POTASSCO_HAS_STATIC_ASSERT has been set to 0 and therefore we define a static_assert macro with two parameters.
• However, the compiler's min_element.h header contains a static_assert(__is_callable<_Compare, decltype(*__first), decltype(*__first)> , which when parsed as a macro, has more than two arguments (due to the commas in the expression). Hence, one would need a variadic static_assert macro.
• Now, my assumption would be that we are in the context of LLVM's LIBCPP, which defines static_assert as a variadic macro in C++98/03 mode.
• But for some reason I currently don't remember, our check contains defined(static_assert) && !defined(_LIBCPP_VERSION) . Hence, we overwrite libcpp's macro definition.

From my current understanding, I'd say that our check in platform.h is broken and we should either just drop the && !defined(_LIBCPP_VERSION) part completely or at least remove the !.

from clingo.

@nuernbergk Could you maybe check whether compiling with DCLASP_BUILD_WITH_THREADS=Off works if you change the defined(static_assert) && !defined(_LIBCPP_VERSION) check in line 31 of potassco/platform.h to defined(static_assert) && defined(_LIBCPP_VERSION) (i.e. just drop the !)?

from clingo.

@BenKaufmann Yep, that fixes it.

from clingo.

I suspect this also causes my build issues in domoritz/clingo-wasm#348. Do you plan to release a new version that can compile without threads (which is what makes sense for WASM at the moment)?

from clingo.