mdspan icon indicating copy to clipboard operation
mdspan copied to clipboard

Issues with Clang 6.0 on Linux when using '-std=c++14 -stdlib=libc++'

Open KineticTheory opened this issue 5 years ago • 2 comments

On Linux, when using Clang++ 6.0 with the compile options -std=c++14 -stdlib=libc++, the CPP macros __cpp_lib_* used in config.hpp are not available. I needed to copy the logic used for _MDSPAN_COMPILER_APPLECLANG to allow this implementation to work.

In layout_right.hpp, I also needed to avoid a double inline

truct layout_right_idx_conditional {
  MDSPAN_INLINE_FUNCTION_DEFAULTED
  constexpr layout_right_idx_conditional() noexcept = default;
  MDSPAN_FORCE_INLINE_FUNCTION
-   constexpr inline bool operator()(size_t Idx, size_t N) const noexcept {
+   constexpr /*inline*/ bool operator()(size_t Idx, size_t N) const noexcept {
    return Idx > N;
  };
};

and in macros.hpp, I needed to change an #if to an #ifdef

//==============================================================================
// <editor-fold desc="inline variables"> {{{1

-#if _MDSPAN_USE_INLINE_VARIABLES
+#ifdef _MDSPAN_USE_INLINE_VARIABLES
#define _MDSPAN_INLINE_VARIABLE inline
#else
#define _MDSPAN_INLINE_VARIABLE
#endif

KineticTheory avatar Dec 30 '19 02:12 KineticTheory

Here is the full patch file that I am using:

diff --git a/include/experimental/__p0009_bits/config.hpp b/include/experimental/__p0009_bits/config.hpp
index 2f9031e..265bd39 100644
--- a/include/experimental/__p0009_bits/config.hpp
+++ b/include/experimental/__p0009_bits/config.hpp
@@ -65,6 +65,14 @@
 #  define _MDSPAN_COMPILER_APPLECLANG
 #endif
 
+#ifdef __clang_version__
+   // This extra logic is required when using LLVM's STL implementation via
+   // compile options '-std=c++14 -stdlib=libc++'.  Using '__clang_version__'
+   // is a big hammer, but I couldn't find a way to to know if we are using
+   // LLVM's libc++.
+#  define _MDSPAN_COMPILER_LLVMCLANG
+#endif
+
 #ifndef __has_cpp_attribute
 #  define __has_cpp_attribute(x) 0
 #endif
@@ -130,6 +138,10 @@
      // appleclang seems to be missing the __cpp_lib_... macros, but doesn't seem to lie about C++14 making
      // integer_sequence work
 #    define _MDSPAN_USE_INTEGER_SEQUENCE 1
+#  elif defined(_MDSPAN_COMPILER_LLVMCLANG) && MDSPAN_HAS_CXX_14
+     // appleclang seems to be missing the __cpp_lib_... macros, but doesn't seem to lie about C++14 making
+     // integer_sequence work
+#    define _MDSPAN_USE_INTEGER_SEQUENCE 1
 #  endif
 #endif
 
@@ -152,6 +164,9 @@
 #  elif defined(_MDSPAN_COMPILER_APPLECLANG) && MDSPAN_HAS_CXX_14
      // appleclang seems to be missing the __cpp_lib_... macros, but doesn't seem to lie about C++14
 #    define _MDSPAN_USE_STANDARD_TRAIT_ALIASES 1
+#  elif defined(_MDSPAN_COMPILER_LLVMCLANG) && MDSPAN_HAS_CXX_14
+     // appleclang seems to be missing the __cpp_lib_... macros, but doesn't seem to lie about C++14
+#    define _MDSPAN_USE_STANDARD_TRAIT_ALIASES 1
 #  endif
 #endif
 
diff --git a/include/experimental/__p0009_bits/layout_right.hpp b/include/experimental/__p0009_bits/layout_right.hpp
index 917f457..40d0b41 100644
--- a/include/experimental/__p0009_bits/layout_right.hpp
+++ b/include/experimental/__p0009_bits/layout_right.hpp
@@ -59,7 +59,7 @@ struct layout_right_idx_conditional {
   MDSPAN_INLINE_FUNCTION_DEFAULTED
   constexpr layout_right_idx_conditional() noexcept = default;
   MDSPAN_FORCE_INLINE_FUNCTION
-  constexpr inline bool operator()(size_t Idx, size_t N) const noexcept {
+  constexpr /*inline*/ bool operator()(size_t Idx, size_t N) const noexcept {
     return Idx > N;
   };
 };
diff --git a/include/experimental/__p0009_bits/macros.hpp b/include/experimental/__p0009_bits/macros.hpp
index fe106d2..23f4bd4 100644
--- a/include/experimental/__p0009_bits/macros.hpp
+++ b/include/experimental/__p0009_bits/macros.hpp
@@ -211,7 +211,7 @@
 //==============================================================================
 // <editor-fold desc="inline variables"> {{{1
 
-#if _MDSPAN_USE_INLINE_VARIABLES
+#ifdef _MDSPAN_USE_INLINE_VARIABLES
 #  define _MDSPAN_INLINE_VARIABLE inline
 #else
 #  define _MDSPAN_INLINE_VARIABLE

I didn't create a PR because I don't fully understand the implications of some of these changes (especially the hack for LLVM's libc++ and commenting out inline). There is probably a much more elegant way to detect if LLVM's stdlib option is being used.

BTW, I also needed to suppress compile warnings with the following:

#ifdef _MSC_FULL_VER
// - 4348: redefinition of default parameter
#pragma warning(push)
#pragma warning(disable : 4348)
#endif

#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunused-parameter"
#pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments"
#endif

#include <mdspan>

#ifdef __clang__
#pragma clang diagnostic pop
#endif

#ifdef _MSC_FULL_VER
#pragma warning(pop)
#endif

These warnings may or may not be significant to this implementation.

KineticTheory avatar Jan 02 '20 17:01 KineticTheory

We are working on fixing a good chunk of this, thanks for the report. We have not yet decided what the minimum clang version is we gonna support though.

crtrott avatar Mar 03 '20 00:03 crtrott