Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

D new wrapper #2456

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

D new wrapper #2456

wants to merge 1 commit into from

Conversation

TurkeyMan
Copy link
Contributor

No description provided.

@dlang-bot
Copy link
Contributor

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

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 + druntime#2456"

@TurkeyMan TurkeyMan force-pushed the d_new_wrapper branch 2 times, most recently from 94a1d7f to 36d742d Compare January 9, 2019 23:36
@TurkeyMan
Copy link
Contributor Author

TurkeyMan commented Jan 9, 2019

@WalterBright Perhaps you can help me understand what's going on here?
Tests error with unresolved extern to the C++ ::new/delete functions because it doesn't link the C++ runtime.
...but why? There's no calls to these functions anywhere, so they should be stripped, rather than trying to link their extern dependencies.

Now, that issue aside, these functions have no reason to exist in physical form in the first place!
I tried to wrap them in pragma(inline, true) to cause the compiler to not emit code for them... but it seems that doesn't work. Why not? Isn't that exactly what pragma(inline) is meant to do?

I tried an alternative strategy making a dummy template argument, so that I could guarantee no codegen without instantiation; except that lead to this issue: https://issues.dlang.org/show_bug.cgi?id=19569

I'm out of ideas... I've run into this issue a lot with the C++ stuff... I need to write a function that should not gen code unless it's called, and it doesn't always need to be a template. I thought that's what pragma(inline) was meant to address, no?

@jacob-carlborg
Copy link
Contributor

On Posix all symbols are visible outside of the object files. There's nothing stopping code in other object files from using that symbol. Therefore code most be generated.

@TurkeyMan
Copy link
Contributor Author

An inline shouldn't be code until it's called... it should be like a template, but without the T.

@rainers
Copy link
Member

rainers commented Jan 10, 2019

Whether pragma(inline,true) actually causes an error if the function is not inlined is currently "implementation defined". If it is made an error the function never has to be generated, but I don't think this is currently done (it should also be an error to take the address the function then).

The build errors here are from building the shared library that exports all symbols, so none can be removed by the linker.

@TurkeyMan
Copy link
Contributor Author

TurkeyMan commented Jan 10, 2019

even if it doesn't actually inline for whatever reason and does get generated, the compiler could emit the function on the first reference, rather than doing so eagerly.

Point is, implementation details aside, the semantic patterns don't actually match.

The inline function shouldn't exist in the .so unless it was called.

@TurkeyMan
Copy link
Contributor Author

@ghost
Copy link

ghost commented Mar 3, 2019

Seems like a mistake to allow overloads that only differ in that they are nothrow. In C++ you can't do this and it seems D makes no effort to try to resolve which function should be called based on the current scope. Even if it did, it would be an ambiguous call in scopes that aren't marked nothrow. Using a different name for the nothrow overload instead will solve the problem, or pass a nothrow object like the C++ version.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs Rebase needs a `git rebase` performed Needs Work stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants