Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 19949: Implement support for C++ ABI tags #10927

Merged
merged 4 commits into from
Mar 28, 2020

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Mar 15, 2020

Implements C++ ABI tags with a few more restriction than the previous PR (9995).
In particular, do not follow C++ source-level conventions when impractical
(e.g. propagation). Since this is a feature that will be seldom used by users,
I favored implementation simplicity over user-friendly semantics.

Previous PR by @SSoulaimane : #9995
Since the author has gone AWOL and there were some discussions on the initial PR, I had a go at implementing it. I mostly used the tests from the original PR, which were very comprehensive, but tried to keep the special casing in semantic to a minimum.
Requires dlang/druntime#2990 to pass tests.

@Geod24 Geod24 requested a review from ibuclaw as a code owner March 15, 2020 23:22
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @Geod24!

Bugzilla references

Auto-close Bugzilla Severity Description
19949 normal C++ Mangling: no support for abi-tags from the Itanium ABI

Testing this PR locally

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

dub run digger -- build "master + dmd#10927"

@thewilsonator
Copy link
Contributor

restruction? Restriction?

src/dmd/semantic2.d Show resolved Hide resolved
src/dmd/cppmangle.d Outdated Show resolved Hide resolved
@Geod24
Copy link
Member Author

Geod24 commented Mar 16, 2020

So on OSX, the compiler is too old:

HOST_CXX (c++): Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn)
Target: x86_64-apple-darwin13.4.0
Thread model: posix

From a quick Google, only LLVM 3.9 knows about ABI tags.
3.9 was release 2016-09-02, 3.5.0 was released 2014-01-02 and 3.5.2 was released 2015-04-02.

I tested it on my machine, it works, but there's no way to test it on the auto-tester.

@Geod24 Geod24 force-pushed the cpp_abi_tag branch 3 times, most recently from b0f3ae0 to a361021 Compare March 17, 2020 05:12
@Geod24
Copy link
Member Author

Geod24 commented Mar 17, 2020

Okay that's not going to work. Maybe it's time to have another CI provider run the C++ interop tests.

@MoonlightSentinel
Copy link
Contributor

Okay that's not going to work. Maybe it's time to have another CI provider run the C++ interop tests.

Or manually let the test suite fetch a more recent clang if necessary?

@Geod24
Copy link
Member Author

Geod24 commented Mar 25, 2020

Okay that's not going to work. Maybe it's time to have another CI provider run the C++ interop tests.

#10964

Geod24 added 2 commits March 27, 2020 13:42
The mangling issues have long been resolved,
and osx32 has been dropped.
This should make our life so much simpler.
@Geod24 Geod24 force-pushed the cpp_abi_tag branch 4 times, most recently from 30ba5cf to 700d467 Compare March 27, 2020 09:27
posix.mak Outdated Show resolved Hide resolved
*/
void printCppSources (in const(char)[][] compiled)
{
version (Posix)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The windows machines have some ported Posix utilities AFAICT (e.g. bash). Might be worth to try nm and silently ignore failures on windows.

@Geod24
Copy link
Member Author

Geod24 commented Mar 27, 2020

Runnable_cxx also needs to be disabled on semaphore ci because it uses GCC 4.8.4

Geod24 and others added 2 commits March 28, 2020 05:09
This is now handled by Github actions. Don't disable Windows because this hasn't been implemented yet.
…nium ABI

Implements C++ ABI tags with a few more restriction than the previous PR (9995).
In particular, do not follow C++ source-level conventions when impractical
(e.g. propagation). Since this is a feature that will be seldom used by users,
I favored implementation simplicity over user-friendly semantics.

Co-Authored-By: سليمان السهمي  (Suleyman Sahmi) <sahmi.soulaimane@gmail.com>
@Geod24
Copy link
Member Author

Geod24 commented Mar 27, 2020

Green at last 🎉

@thewilsonator thewilsonator merged commit 371de31 into dlang:master Mar 28, 2020
@Geod24 Geod24 deleted the cpp_abi_tag branch March 28, 2020 04:23
@Geod24
Copy link
Member Author

Geod24 commented Mar 28, 2020

Hum, this went red for Ubuntu after merge... Will fix.

@Geod24
Copy link
Member Author

Geod24 commented Mar 28, 2020

Fix being worked on here: #10974

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants