Fix Issue 18958 - extern(C++) wchar, dchar mangling not correct

[TurkeyMan]

No description provided.

[dlang-bot]

Thanks for your pull request and interest in making D better, @TurkeyMan! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18958	normal	extern(C++) wchar, dchar mangling not correct

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8342"

[Geod24]

Does this mean we drop compatibility for C++ < 11 ?

[Geod24]

Note there's https://issues.dlang.org/show_bug.cgi?id=14956

[TurkeyMan]

C++ < 11 doesn't have wchar/dchar (ie, char16_t/char32_t). It does have wchar_t, except we mangle that wrong too.
I think we need to typedef wchar_t so it's a distinct type, and then mangle it properly.
This shouldn't affect that, since we're testing that the D types match the C++11 types, and they never actually mapped to any <11 types (even though there was a nasty hack in place).

[TurkeyMan]

error: ‘char16_t’ does not name a type

I saw that coming a mile off!

Does anyone know how we build the C++ file with -std=c++11?

[Geod24]

Don't think that's possible yet

[TurkeyMan]

Well it's about time :)

[TurkeyMan]

Feel like negotiating/working on a solution to that problem?
I need to go to bed! :P

[jacob-carlborg]

Does anyone know how we build the C++ file with -std=c++11?

As far a I know, DMC does not support C++11.

[wilzbach]

You could create a new test file, add the c11 flag and ignore that test for win32?

[TurkeyMan]

DMC can keep its mangling hacks then, and typedef char16_t/char32_t to some int types?

[TurkeyMan]

and ignore that test for win32?

What's wrong with win32?

How do I apply the c++11 flag?

[wilzbach]

How do I apply the c++11 flag?

Here's one example:

diff --git a/test/compilable/extra-files/test18958.cpp b/test/compilable/extra-files/test18958.cpp
new file mode 100644
index 000000000..45e1ce1e9
--- /dev/null
+++ b/test/compilable/extra-files/test18958.cpp
@@ -0,0 +1,5 @@
+#include <tuple>
+int hello() {
+    std::tuple<int, double> t(1, 2.0);
+    return std::get<0>(t);
+}
diff --git a/test/compilable/extra-files/test18958.d b/test/compilable/extra-files/test18958.d
new file mode 100644
index 000000000..7da3a94ec
--- /dev/null
+++ b/test/compilable/extra-files/test18958.d
@@ -0,0 +1,5 @@
+extern(C++) int hello();
+
+void main(){
+    assert(hello() == 1);
+}
diff --git a/test/compilable/test18958.sh b/test/compilable/test18958.sh
new file mode 100644
index 000000000..bfaf3cabf
--- /dev/null
+++ b/test/compilable/test18958.sh
@@ -0,0 +1,8 @@
+// DISABLED: win32
+#!/usr/bin/env bash
+
+"${CC}" -std=c++11 -o ${OUTPUT_BASE}cpp${OBJ} -c ${EXTRA_FILES}/${TEST_NAME}.cpp
+
+$DMD -c -m${MODEL} -of${OUTPUT_BASE}${EXE} -I${EXTRA_FILES} ${EXTRA_FILES}/${TEST_NAME}.d ${OUTPUT_BASE}cpp${OBJ}
+
+rm -f ${OUTPUT_BASE}${EXE} ${OUTPUT_BASE}cpp${OBJ}
diff --git a/test/tools/exported_vars.sh b/test/tools/exported_vars.sh
index 4f097e92d..5ca796cc0 100644
--- a/test/tools/exported_vars.sh
+++ b/test/tools/exported_vars.sh
@@ -12,3 +12,5 @@ if [ "$OS" == "win32" ] || [ "$OS" == "win64" ]; then
 else
     export LIBEXT=.a
 fi
+
+export CC="${CC:-c++}" # C++ compiler to use

An alternative to this would be to add sth. like CFLAGS to d_do_test.

What's wrong with win32?

I thought DigitalMars C++ doesn't support C++11?

[TurkeyMan]

...no. We already clearly demonstrate that we can't maintain even a single abi revision.
We should just support the 'current' one.

[ibuclaw]

We should just support the 'current' one.

It starts off with no one agreeing what 'current' means. I don't think cppmangle even does C++11 Std strings, even though I raised that bug report probably 2 years ago now. (this is why we can't maintain a single version :-)

At least on the gcc side, abi versions are small additive changes. I can't say that maintaining it would be beyond us.

Also, at least I will have a dependency on C++98 for the foreseeable future, so will end up adding an -stdc++=xx style option anyway even if the frontend doesn't. :-(

[TurkeyMan]

Okay. Well, don't get me wrong, I'm not against it! If you can propose how we make it work uniformly?

[TurkeyMan]

It would be really great to move this forward.

[Geod24]

@TurkeyMan : You can use // CXXFLAGS: -std=c++11 to compile your EXTRA_CPP_SOURCES with this flag.

[ibuclaw]

You can use // CXXFLAGS: -std=c++11 to compile your EXTRA_CPP_SOURCES with this flag.

This just hides that C++98 support is now broken. I won't treat that as anything else other than a regression.

[TurkeyMan]

I agree in principle, but we're in a broken position either way.
I think there are 2 options here; we officially support just one C++ version, and in that case, people have generally agreed it should be the 'current' one, or, we add the concept of C++ version to D, and I haven't heard any consensus, or even support for that.

I think The right thing to do would be, in the meantime while we only do support one version, declare that we support the 'current' version rather than a random 'whichever one seems to work mix', which feels like the only reasonable option. And then in the future if we decide to support C++ versions selectively, then we can include multi-version tests and commit to backwards compatibility then... No?

[thewilsonator]

@TurkeyMan The linux problem seems to have been solved. All that needs doing is fixing the flags passed to cl and versioning out dmc.

[TurkeyMan]

Good :)

[WalterBright]

Still needs changelog.

[jacob-carlborg]

Still needs changelog.

@WalterBright the PR is linked with issue https://issues.dlang.org/show_bug.cgi?id=18958. It will automatically generate a changelog entry. I don't think we need an explicit changelog entry describing the the fix.

[thewilsonator]

(again)

Bug report will generate a changelog entry.

[thewilsonator]

Wonderful VS2013 doesn't support char16_t/char32_t

[TurkeyMan]

You're loving it!
This has been my eternal punishment for trying to do any of the extern(C++) stuff!

[thewilsonator]

I F$%#^# HATE THIS CI SYSTEM!

[wilzbach]

Well, we all would love to replace the Auto-Tester, but so far the problem was always money. Andrei recently told me that the DLF these days would pay for their own CI machines, but unfortunately I don't really have time to the migration these days.
If someone is interested in this, ping me. I'm happy to help. The general idea is pretty simple though: we rent a few cheap Windows/OSX boxes, install tooling and the Buildkite agent and then use them to fully cover all platforms, s.t. we can retire the Auto-Tester.

[thewilsonator]

To correct my self slightly, I hate the windows machines because I hate DMC and I hate Microsoft for being inconsistent with their C++11 support on VS2013. We axed OSX32, that made things much nicer.

Could we just move to Azure for the Windows stuff?

[wilzbach]

Could we just move to Azure for the Windows stuff?

Yes, but we would need to configure it for druntime and Phobos (it's just running the same script though with just the repository cloning being different).