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

Split C++11-specific tests in their own file #9185

Merged
merged 3 commits into from
Jan 7, 2019

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Jan 3, 2019

Follow up on the eager merge of #9138

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @Geod24!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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#9185"

@thewilsonator
Copy link
Contributor

Thanks!

@Geod24
Copy link
Member Author

Geod24 commented Jan 3, 2019

Sweet, it does not work (except on Linux and MacOS ofc).
FreeBSD:

 ... runnable/cpp11.d               -stdc++=c++11  -fPIC -L-lstdc++ (-O -inline -release)
Test runnable/cpp11.d failed.  The logged output:
/home/braddr/sandbox/at-client/pull-3468076-FreeBSD_64_64/dmd/generated/freebsd/release/64/dmd -conf= -m64 -Irunnable -stdc++=c++11  -fPIC -L-lstdc++  -odgenerated/runnable -ofgenerated/runnable/cpp11_0  runnable/cpp11.d generated/runnable/cpp11.cpp.o 
generated/runnable/cpp11_0.o: In function `_D5cpp116test17FZv':
runnable/cpp11.d:(.text._D5cpp116test17FZv+0x7): undefined reference to `_Z8testnullDn'
runnable/cpp11.d:(.text._D5cpp116test17FZv+0x11): undefined reference to `_Z12testnullnullDnDn'
cc: error: linker command failed with exit code 1 (use -v to see invocation)

Windows:

 ... runnable\cpp11.d               -stdc++=c++11   (-inline -release -g -O)
Test runnable\cpp11.d failed.  The logged output:
..\generated\windows\release\32\dmd.exe -conf= -m64 -Irunnable -stdc++=c++11   -L/OPT:NOICF  -odtest_results\runnable -oftest_results\runnable\cpp11_0.exe  runnable\cpp11.d test_results\runnable\cpp11.cpp.obj  -map nul.map


cpp11_0.obj : error LNK2019: unresolved external symbol "void __cdecl testnullnull(std::nullptr_t,std::nullptr_t)" (?testnullnull@@YAX$$T$$T@Z) referenced in function _D5cpp116test17FZv



test_results\runnable\cpp11_0.exe : fatal error LNK1120: 1 unresolved externals



Error: linker exited with status 1120

@ibuclaw do you want to take a look at it or shall I ?

@ibuclaw
Copy link
Member

ibuclaw commented Jan 4, 2019

I have no care for the windows mangling (itanium is used for all platforms including windows my end).

I have a FreeBSD VM around somewhere, however my initial assumption is that the version of gcc installed on the box is old (I think gcc 4.7 or older)

See my comment on abi timeline of nullptr_t.
#9130 (comment)

@ibuclaw
Copy link
Member

ibuclaw commented Jan 4, 2019

You could try setting false if FreeBSD in the new target hook to verify.

if (isFreeBSD)
{
  substitute = false;
  return true;
} 

@rainers
Copy link
Member

rainers commented Jan 4, 2019

cpp11_0.obj : error LNK2019: unresolved external symbol "void __cdecl testnullnull(std::nullptr_t,std::nullptr_t)" (?testnullnull@@Yax$$T$$T@Z) referenced in function _D5cpp116test17FZv

VC uses back references for std::nullptr_t, too, so adding this to VisualCPPHndler.visit(TypeNull) should help:

+++ src/dmd/cppmanglewin.d
@@ -137,6 +137,8 @@ public:
     {
         if (checkImmutableShared(type))
             return;
+        if (checkTypeSaved(type))
+            return;
 
         buf.writestring("$$T");
         flags &= ~IS_NOT_TOP_TYPE;

When it comes to C++ interop, many bugs are mangling related.
As we add more and more tests involving the STL,
the linker errors become more advance and subtle,
and getting those on a platform one does not have access to (e.g. FreeBSD)
is usually hard to debug, as we deal with different compiler/stdlib versions.
@Geod24 Geod24 force-pushed the cpp-test-fixes branch 2 times, most recently from 7a59a88 to 9bba34e Compare January 7, 2019 05:30
@thewilsonator
Copy link
Contributor

This is all green. Good to go?

@Geod24
Copy link
Member Author

Geod24 commented Jan 7, 2019

Well the test is disabled on most platforms, but this is why you should not merge commented out tests, they rarely work.
But yeah, let's go ahead and get this merged, given the bandwidth on C++1X I assume this will see use fairly soon.

@thewilsonator
Copy link
Contributor

And that first commit should make it easier for Manu to do remote debugging.

@thewilsonator
Copy link
Contributor

Auto-merge toggled on

@thewilsonator thewilsonator merged commit 2d2af29 into dlang:master Jan 7, 2019
@Geod24 Geod24 deleted the cpp-test-fixes branch January 7, 2019 10:42
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.

5 participants