Skip to content

Mangle extern(C++) ctor as C++ ctor.#5884

Closed
redstar wants to merge 1 commit intodlang:masterfrom
redstar:cppctor
Closed

Mangle extern(C++) ctor as C++ ctor.#5884
redstar wants to merge 1 commit intodlang:masterfrom
redstar:cppctor

Conversation

@redstar
Copy link
Contributor

@redstar redstar commented Jun 26, 2016

Additionally, it disallows a extern(C++) ctor to have a body.
It should be safe to call a C++ ctor from C++ and D as long as
the initialization rules from C++ are used. Disallowing an ctor
body ensures this.

@mathias-lang-sociomantic
Copy link
Contributor

So this is only for interfacing with foreign C++ code, not for C++ to interface with D code ?
I'm pretty sure we'll get soon enough a request of to do the later.

Anyway, a bit of self-promotion: https://forum.dlang.org/post/576C770B.4060408@sociomantic.com ;)

@WalterBright
Copy link
Member

Additionally, it disallows a extern(C++) ctor to have a body.

I agree with @mathias-lang-sociomantic and this restriction should not be added. Compatibility goes both ways.

@redstar
Copy link
Contributor Author

redstar commented Jun 28, 2016

My ultimate goal is to add interfacing for both ways. I added the restriction only to make it possible to split the development into two steps. I still try to figure out where to change the initialization of a class/struct.

@mathias-lang-sociomantic I already work on a PR for the language chance.

@redstar
Copy link
Contributor Author

redstar commented Jun 28, 2016

I removed the restriction. But without changing the initialization, this is a bit dangerous.

@mathias-lang-sociomantic I didn't noticed that @JohanEngelen already created a PR for the spec change!

@mathias-lang-sociomantic
Copy link
Contributor

@mathias-lang-sociomantic I didn't noticed that @JohanEngelen already created a PR for the spec change!

You mean dlang/dlang.org#1397 ? That's only for extern(C++, {class,struct{). Or did I miss one ?

@redstar redstar force-pushed the cppctor branch 2 times, most recently from a74c461 to 828cd4f Compare June 28, 2016 17:16
@redstar redstar force-pushed the cppctor branch 2 times, most recently from a500c9a to ff3cb3c Compare July 5, 2016 06:29
kinke added a commit to kinke/druntime that referenced this pull request Dec 9, 2016
Implementing them (wrongly btw, ignoring the msg) leads to an implicit call to the `super` ctor of base class `core.stdcpp.exception.std.exception`, which is only declared in D. Even if the ctor signature was correct (see below), it's mangled differently (see dlang/dmd#5884), so there'd be an unresolved external anyway. So don't implement it, just declare it (also done for Glibc) to defer the linking error.

Quality-wise, the 2 `core.stdcpp` modules are at most at an alpha stage for MSVCRT. Other issues I've noted so far using **VS 2015**:
* The signature of the `core.stdcpp.exception.std.exception.this()` ctor is wrong. There's no default, i.e., parameter-less one, only `std::exception::exception(const char *_Message = "unknown", int x=1)`.
* `std::exception` only contains a pointer, no additional `bool` field.
* The signature of `core.stdcpp.typeinfo.std.type_info.name()` is wrong. It doesn't take any arguments.

The first 2 aren't crucial, as these C++ exceptions are most likely never constructed on the D side. But the last one will need to be fixed. @rainers: Which VS versions does DMD currently support?
Additionally, it disallows a extern(C++) ctor to have a body.
It should be safe to call a C++ ctor from C++ and D as long as
the initialization rules from C++ are used. Disallowing an ctor
body ensures this.
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @redstar! 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

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

@rainers
Copy link
Member

rainers commented Sep 2, 2018

This has been part of #8265, I guess we can close this one.

@rainers rainers closed this Sep 2, 2018
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