druntime icon indicating copy to clipboard operation
druntime copied to clipboard

D `new` wrapper

Open TurkeyMan opened this issue 6 years ago • 8 comments

TurkeyMan avatar Jan 09 '19 23:01 TurkeyMan

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"

dlang-bot avatar Jan 09 '19 23:01 dlang-bot

@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?

TurkeyMan avatar Jan 09 '19 23:01 TurkeyMan

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.

jacob-carlborg avatar Jan 10 '19 07:01 jacob-carlborg

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

TurkeyMan avatar Jan 10 '19 07:01 TurkeyMan

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.

rainers avatar Jan 10 '19 07:01 rainers

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 avatar Jan 10 '19 07:01 TurkeyMan

This is blocked on: https://issues.dlang.org/show_bug.cgi?id=19570 https://github.com/dlang/dmd/pull/9235

TurkeyMan avatar Jan 12 '19 21:01 TurkeyMan

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.

ghost avatar Mar 03 '19 17:03 ghost