Skip to content

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented May 10, 2013

Fixes http://d.puremagic.com/issues/show_bug.cgi?id=10058

Though maybe the other implementations of ::toCppMangle should be altered to have the same logic as the change here?

Currently it's a half/half situation.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 10, 2013

Hold fire on this, am investigating a little more... see the bug report.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 10, 2013

Probably should add [DDMD] to this, as I think this might block the ability to have a visitor interface between D and C++ glue, if said visitor interface is defined as extern(C++)

@yebblies
Copy link
Contributor

Add the test cases! static assert(s.mangleof == "...") works for you linux people!

@ibuclaw
Copy link
Member Author

ibuclaw commented May 10, 2013

Should probably produce a bunch of test cases for this to make sure that D follows C++ mangling correctly.

@yebblies
Copy link
Contributor

I'd be happy with just the ones from the bug report for now.

Some are available here: https://github.com/mirrors/gcc/tree/master/gcc/testsuite/g++.dg/abi

@ibuclaw
Copy link
Member Author

ibuclaw commented May 11, 2013

Didn't see that you commented before me. :)

I'm in London today and without a laptop. Will fix this up either tonight or tomorrow. Most of what I can see in the g++ testsuite are to do with C++ templates...

@ibuclaw
Copy link
Member Author

ibuclaw commented May 13, 2013

Waiting for auto-tester.

@yebblies
Copy link
Contributor

Wait - how come this test passes on win32?

@ibuclaw
Copy link
Member Author

ibuclaw commented May 13, 2013

I don't think the autotester has ran yet?

Did not check win32. :)

@ghost
Copy link

ghost commented May 13, 2013

I've edited the title so it's easier to know what the pull is about.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 13, 2013

typo in one of the static asserts... serves me right for typing the mangling by hand... :)

@ibuclaw
Copy link
Member Author

ibuclaw commented May 13, 2013

ok, linux testers have started passing... did you say that this won't work for windows?

@yebblies
Copy link
Contributor

... Or OSX... cppmangle.c is not even part of the windows build. On OSX you could check the correct OSX mangling is generated... but on windows it's done in the backend and cannot be checked via mangleof. And of course the mangling is different from linux.

@yebblies
Copy link
Contributor

I figure a version(Linux): at the top will fix it for now.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 14, 2013

ok, version(linux) is it then...

yebblies added a commit that referenced this pull request May 15, 2013
Fix Issue 10058 - Inconsistent mangling between C++ and extern(C++).
@yebblies yebblies merged commit 60dee0b into dlang:master May 15, 2013
@WalterBright
Copy link
Member

This breaks the following code:

../dmd -m32 -c cppa
g++ -m32 -c cppb.cpp

g++ -m32 cppa.o cppb.o -o cppa -m32 -lphobos2 -lpthread -lm -L../phobos/generated/osx/release/32
Undefined symbols for architecture i386:
  "__Z5foo10PKcS1_", referenced from:
      _D4cppa6test10FZv in cppa.o
  "__Z7foobar9P5elem9S1_", referenced from:
      _D4cppa5test9FZv in cppa.o
ld: symbol(s) not found for architecture i386
collect2: ld returned 1 exit status

--- errorlevel 256

(failure was on OSX)

WalterBright added a commit to WalterBright/dmd that referenced this pull request Jun 1, 2013
This reverts commit 60dee0b, reversing
changes made to ad0679d.
@IgorStepanov
Copy link
Contributor

@WalterBright please, write contents of cppb.cpp and cppa.d

@yebblies
Copy link
Contributor

yebblies commented Jun 1, 2013

@WalterBright Thanks, please put it in bugzilla with a test case (using .mangleof if possible)

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 1, 2013

@yebblies I'm on it. =)

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 1, 2013

ibuclaw pushed a commit to ibuclaw/dmd that referenced this pull request Jun 1, 2013
WalterBright added a commit that referenced this pull request Jun 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants