Merge pull request #2719 from TurkeyMan/string_on_gcc_modern

Fix string tests not running
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>

Author: dlang-bot

changelog/std_string.dd

@@ -1,5 +1,9 @@
 Added `core.stdcpp.string`.
 
-Added `core.stdcpp.string`, which links against C++ `std::string`
+Added `core.stdcpp.string`, which links against C++ `std::basic_string`
 
-Currently only supported and tested for the Visual Studio C++ runtime library. Other runtimes coming soon(tm)!
+Known issues:
+
+Currently, the GCC (`libstdc++`) implementation has a known issue with the modern `__cxx11` ABI, because the C++ struct contains an interior pointer which is illegal in D, and incompatible with D move semantics.
+To use `core.stdcpp.string` on linux/GCC, you must use the old string ABI by supplying `-D_GLIBCXX_USE_CXX11_ABI=0` to `g++`, and also `-version=_GLIBCXX_USE_CXX98_ABI` to your D compiler.
+Work to define D move constructors is ongoing and expected to resolve this issue when it arrives.

src/core/stdcpp/string.d

@@ -13,37 +13,42 @@
 module core.stdcpp.string;
 
 import core.stdcpp.allocator;
+import core.stdcpp.xutility : StdNamespace;
 import core.stdc.stddef : wchar_t;
 
+version (OSX)
+{
+    // Apple decided to rock a different ABI... good for them!
+    version = _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT;
+}
 version (CppRuntime_Gcc)
 {
     version (_GLIBCXX_USE_CXX98_ABI)
     {
-        private enum StdNamespace = "std";
+        private enum StringNamespace = "std";
 //        version = __GTHREADS; // TODO: we need to make ref-count interactions atomic
     }
     else
     {
         import core.internal.traits : AliasSeq;
-        private enum StdNamespace = AliasSeq!("std", "__cxx11");
+        private enum StringNamespace = AliasSeq!("std", "__cxx11");
     }
 }
 else
-    import core.stdcpp.xutility : StdNamespace;
+    alias StringNamespace = StdNamespace;
 
 enum DefaultConstruct { value }
 
 /// Constructor argument for default construction
 enum Default = DefaultConstruct();
 
-extern(C++, (StdNamespace)):
 @nogc:
 
 /**
  * Character traits classes specify character properties and provide specific
  * semantics for certain operations on characters and sequences of characters.
  */
-struct char_traits(CharT) {}
+extern(C++, (StdNamespace)) struct char_traits(CharT) {}
 
 // I don't think we can have these here, otherwise symbols are emit to druntime, and we don't want that...
 //alias std_string = basic_string!char;
@@ -57,6 +62,7 @@ struct char_traits(CharT) {}
  * C++ reference: $(LINK2 https://en.cppreference.com/w/cpp/string/basic_string)
  */
 extern(C++, class)
+extern(C++, (StringNamespace))
 struct basic_string(T, Traits = char_traits!T, Alloc = allocator!T)
 {
 extern(D):
@@ -727,6 +733,8 @@ extern(D):
         }
         else
         {
+            pragma(msg, "libstdc++ std::__cxx11::basic_string is not yet supported; the struct contains an interior pointer which breaks D move semantics!");
+
             //----------------------------------------------------------------------------------
             // GCC/libstdc++ modern implementation
             //----------------------------------------------------------------------------------
@@ -1309,8 +1317,8 @@ private:
 version (CppRuntime_Microsoft)
 {
     import core.stdcpp.xutility : _ITERATOR_DEBUG_LEVEL;
-extern(C++, "std"):
 
+extern(C++, (StdNamespace)):
     extern (C++) struct _String_base_types(_Elem, _Alloc)
     {
         alias Ty = _Elem;

test/stdcpp/Makefile

@@ -6,23 +6,22 @@ TESTS:=allocator new
 TESTS11:=array
 TESTS17:=string_view
 OLDABITESTS:=
-LIBCPPTESTS:=
 
 ifeq (osx,$(OS))
-	TESTS+=string
+#	TESTS+=string
 #	TESTS+=vector
 endif
 ifeq (linux,$(OS))
-#	TESTS+=string vector
+#	TESTS+=string
+#	TESTS+=vector
 	OLDABITESTS+=string
-#	OLDABITESTS+=vector
-#	LIBCPPTESTS:=string
 endif
 ifeq (freebsd,$(OS))
 	TESTS+=string
 #	TESTS+=vector
 endif
 
+# some build machines have ancient compilers, so we need to disable C++17 tests
 ifneq (yes,$(HASCPP17))
 TESTS17:=
 endif
@@ -59,11 +58,6 @@ $(ROOT)/%_old.done : $(ROOT)/%_old
 	@echo Testing $*_old
 	$(QUIET)$(TIMELIMIT)$(ROOT)/$*_old $(RUN_ARGS)
 	@touch $@
-# run libc++ tests with GCC
-$(ROOT)/%_libcpp.done : $(ROOT)/%_libcpp
-	@echo Testing $*_libcpp
-	$(QUIET)$(TIMELIMIT)$(ROOT)/$*_libcpp $(RUN_ARGS)
-	@touch $@
 
 # build C++98 tests
 $(ROOT)/%: $(SRC)/%.cpp $(SRC)/%_test.d
@@ -85,11 +79,6 @@ $(ROOT)/%_old: $(SRC)/%.cpp $(SRC)/%_test.d
 	mkdir -p $(dir $@)
 	$(QUIET)$(DMD) $(DFLAGS) -version=_GLIBCXX_USE_CXX98_ABI -main -unittest -c -of=$(ROOT)/$*_old_d.o $(SRC)/$*_test.d
 	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -D_GLIBCXX_USE_CXX11_ABI=0 -o $@ $< $(ROOT)/$*_old_d.o $(DRUNTIME) -lpthread
-# build libc++ tests with GCC
-$(ROOT)/%_libcpp: $(SRC)/%.cpp $(SRC)/%_test.d
-	mkdir -p $(dir $@)
-	$(QUIET)$(DMD) $(DFLAGS) -version=CppRuntime_Clang -main -unittest -c -of=$(ROOT)/$*_libcpp_d.o $(SRC)/$*_test.d
-	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -stdlib=libc++ -o $@ $< $(ROOT)/$*_libcpp_d.o $(DRUNTIME) -lpthread
 
 clean:
 	rm -rf $(GENERATED)

test/stdcpp/src/string_test.d

@@ -7,59 +7,52 @@ alias std_wstring = basic_string!wchar;
 
 unittest
 {
-    version(Windows)
-    {
-        std_string str = std_string("Hello");
-
-        assert(str.size == 5);
-        assert(str.length == 5);
-        assert(str.empty == false);
-
-        assert(sumOfElements_val(str) == 1500);
-        assert(sumOfElements_ref(str) == 500);
-
-        str = "Hello again with a long long string woo";
-        assert(sumOfElements_val(str) == 10935);
-        assert(sumOfElements_ref(str) == 3645);
-
-        std_string str2 = std_string(Default);
-        assert(str2.size == 0);
-        assert(str2.length == 0);
-        assert(str2.empty == true);
-        assert(str2[] == []);
-
-        str2 = std_string("World");
-        assert(str2[] == "World");
-        str = str2;
-        assert(str[] == "World");
-        str2 = "Direct";
-        assert(str2[] == "Direct");
-        assert(str2[2] == 'r');
-        assert(str2[2 .. 5] == "rec");
-        str2[] = "Plonk!";
-        assert(str2[] == "Plonk!");
-        str2[2] = 'a';
-        str2[3 .. 5] = "ne";
-        assert(str2[] == "Plane!");
-        str2[] = 'a';
-        str2[1 .. 5] = 'b';
-        str2[] += 1;
-        str2[1] += 1;
-        str2[2 .. 4] += 2;
-        assert(str2[] == "bdeecb");
-
-        // test local instantiations...
-        // there's no basic_string<char16_t> instantiation in C++
-        std_wstring str3 = std_wstring("Hello"w);
-
-        assert(str3.size == 5);
-        assert(str3.length == 5);
-        assert(str3.empty == false);
-    }
-    else
-    {
-        pragma(msg, "std.string implementation not yet done for linux - gcc/clang");
-    }
+    std_string str = std_string("Hello");
+
+    assert(str.size == 5);
+    assert(str.length == 5);
+    assert(str.empty == false);
+
+    assert(sumOfElements_val(str) == 1500);
+    assert(sumOfElements_ref(str) == 500);
+
+    str = "Hello again with a long long string woo";
+    assert(sumOfElements_val(str) == 10935);
+    assert(sumOfElements_ref(str) == 3645);
+
+    std_string str2 = std_string(Default);
+    assert(str2.size == 0);
+    assert(str2.length == 0);
+    assert(str2.empty == true);
+    assert(str2[] == []);
+
+    str2 = std_string("World");
+    assert(str2[] == "World");
+    str = str2;
+    assert(str[] == "World");
+    str2 = "Direct";
+    assert(str2[] == "Direct");
+    assert(str2[2] == 'r');
+    assert(str2[2 .. 5] == "rec");
+    str2[] = "Plonk!";
+    assert(str2[] == "Plonk!");
+    str2[2] = 'a';
+    str2[3 .. 5] = "ne";
+    assert(str2[] == "Plane!");
+    str2[] = 'a';
+    str2[1 .. 5] = 'b';
+    str2[] += 1;
+    str2[1] += 1;
+    str2[2 .. 4] += 2;
+    assert(str2[] == "bdeecb");
+
+    // test local instantiations...
+    // there's no basic_string<char16_t> instantiation in C++
+    std_wstring str3 = std_wstring("Hello"w);
+
+    assert(str3.size == 5);
+    assert(str3.length == 5);
+    assert(str3.empty == false);
 }