Allow `-preview=in` only with `extern(D|C++)` #12242

Geod24 · 2021-03-01T04:50:28Z

The intent of `-preview=in` is to make `in` the go-to storage class for input parameters.
In doing so, it makes the ABI less obvious (although still deterministic),
by sometimes degrading the parameter from reference to a value if it is better ABI-wise.
This behavior is obviously at odds with non-`extern(D)` functions,
which expect to match a specific ABI, hence `in` parameters can no longer apply
to non-`extern(D)` or `extern(C++)` functions when `-preview=in` is used.

However, as C++, also have a "go to" storage class for input parameters (`const T&`),
`in` can also be applied on `extern(C++)` function in order to bind to `const T&` parameters.
When doing so, `in` will always mean `ref`. This also allows to expose a closer API
for a function than via `const ref`, as `in` will allow to bind rvalues to `const T&`, as in C++.

@kinke @ibuclaw : Any opinion on this ? If this is too much, I can see two other course of action:

Disabling in for anything non-extern(D);
Making in always make scope const with extern(C), never ref, disable for others;

dlang-bot · 2021-03-01T04:50:30Z

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 run digger -- build "master + dmd#12242"

In preparation for dlang/dmd#12242.

The argument is sent to another process, and can contain a pointer. Additionally, there is currently a PR in DMD to forbid using 'in' with extern(C) functions when '-preview=in' is provided (dlang/dmd#12242).

Geod24 · 2021-03-01T07:29:08Z

Maybe the const solution for extern(C) is the best way forward. After all, we allow to pass D arrays to extern(C) functions (https://github.com/dlang/druntime/blob/0125219f894a43d2ab0a7571b690c96e400051ca/src/rt/aApply.d#L21).

The argument is sent to another process, and can contain a pointer. Additionally, there is currently a PR in DMD to forbid using 'in' with extern(C) functions when '-preview=in' is provided (dlang/dmd#12242).

PetarKirov · 2021-03-01T09:38:07Z

Keep in mind that scope is meaningless for non-extern (D) functions that are not implemented in D, as C and C++ compilers won't enforce that the parameter are not escaped, so while in means const scope maybe-ref in D, it should be translated as exactly const ref for C and C++.

kinke · 2021-03-01T09:46:13Z

Disabling in for anything non-extern(D)

I think I'd prefer that for simplicity, otherwise the in semantics get too complicated.

Edit: This also allows to expose a closer API for a function than via 'const ref', as 'in' will allow to bind rvalues to 'const T&', as in C++. Hmm okay that'd be nice indeed...

Geod24 · 2021-03-02T02:31:08Z

Keep in mind that scope is meaningless for non-extern (D) functions that are not implemented in D

Not exactly meaningless, since the compiler will check scope if called from a @safe context. And yes, we have some in druntime.

I think I'd prefer that for simplicity, otherwise the in semantics get too complicated.
[...]
Hmm okay that'd be nice indeed...

That was exactly my thought process. Maybe @ibuclaw can weight in ?

PetarKirov · 2021-03-02T07:36:19Z

compiler will check scope if called from a @safe context

That's actual the main problem. scope parameters allow allow more code in @safe functions, not less. For example, functions can to receive pointers/references to local variables in @safe code via scope parameters. This is under the assumption that the callee is implemented in D and the compiler has verified that it doesn't escape the address of its scope parameters. If the function is implemented in another language, this assumption doesn't hold any more.

This also allows to expose a closer API for a function than via 'const ref', as 'in' will allow to bind rvalues to 'const T&', as in C++.

P.S. I also think this would be nice.

Geod24 · 2021-03-02T07:44:52Z

If the function is implemented in another language, this assumption doesn't hold any more.

The same can be said of other attributes. IMO C++ const's situation is even worse, are we could be forced to apply const to a parameter even though it is mutated.

In preparation for dlang/dmd#12242.

The argument is sent to another process, and can contain a pointer. Additionally, there is currently a PR in DMD to forbid using 'in' with extern(C) functions when '-preview=in' is provided (dlang/dmd#12242).

In preparation for dlang/dmd#12242.

The argument is sent to another process, and can contain a pointer. Additionally, there is currently a PR in DMD to forbid using 'in' with extern(C) functions when '-preview=in' is provided (dlang/dmd#12242).

atilaneves · 2021-03-29T21:05:38Z

I think it makes sense to be able to declare parameters to be scope even in extern(C) code, especially considering that just because functions have that linkage doesn't imply they're actually written in C.

And even if they are, I for one would like to tell the compiler that they're not escaped.

In preparation for dlang/dmd#12242.

The argument is sent to another process, and can contain a pointer. Additionally, there is currently a PR in DMD to forbid using 'in' with extern(C) functions when '-preview=in' is provided (dlang/dmd#12242).

Geod24 · 2022-06-16T23:22:16Z

I think this is finally passing the CI (!)

thewilsonator · 2022-06-17T01:05:30Z

Final ping: @kinke @ibuclaw

dkorpel · 2022-06-17T11:20:02Z

changelog/previewInLink.dd

@@ -0,0 +1,11 @@
+`-preview=in` can now be used with `extern(C++)`, disabled for other non-D linkage
+
+The intent of `-preview=in` is to make `in` the go-to storage class for input parameters in D.


Is this official vision?

Has Walter said those words exactly? Not that I recall. But he did put in, inout (now ref) and out in the language, so yeah I think that's as close as we'd get.

changelog/previewInLink.dd

The intent of `-preview=in` is to make `in` the go-to storage class for input parameters in D. However it is D centric, as it is an enhanced version of `scope const ref`. As non-`extern(D)` functions usually are expected to match a specific ABI, using `in` is hardly a good idea. However, as C++, also have a "go to" storage class for input parameters (`const T&`), `in` can also be applied on `extern(C++)` function in order to bind to `const T&` parameters. This also allows to expose a closer API for a function than via `const ref`, as `in` will allow to bind rvalues to `const T&`, as in C++.

Geod24 · 2022-06-17T15:08:29Z

@dkorpel : Fixed

ibuclaw · 2022-06-18T14:07:42Z

I have nothing against this, would the likes of @atilaneves or @WalterBright wish to have another look?

WalterBright · 2022-06-18T23:20:47Z

src/dmd/typesem.d

+                    case LINK.cpp:
+                        fparam.storageClass |= STC.ref_;
+                        break;
+                    case LINK.default_, LINK.d:


What exactly does in mean for D linkage? Is it different than for C++?

This PR predates our decision to make in always mean ref. I'll correct it in the next PR.

WalterBright · 2022-06-18T23:21:27Z

Does the meaning of in differ for D and C++ linkage?

atilaneves · 2022-06-20T12:42:03Z

Does the meaning of in differ for D and C++ linkage?

I hope not.

Geod24 · 2022-06-20T12:49:17Z

Not after I disable the optimization, which is the next PR.

Geod24 requested review from ibuclaw and kinke March 1, 2021 04:50

Geod24 changed the title ~~-preview=in can now be used with extern(C++), disabled for other non-D linkage~~ Allow -preview=in only with extern(D|C++) Mar 1, 2021

Geod24 force-pushed the preview-in-extern-cpp branch from b3d2cf7 to 2f6e0f6 Compare March 1, 2021 04:51

Geod24 added a commit to Geod24/druntime that referenced this pull request Mar 1, 2021

Remove usage of 'in' on _d_newitemU

fe318ee

In preparation for dlang/dmd#12242.

Geod24 added a commit to Geod24/druntime that referenced this pull request Mar 1, 2021

Remove usage of 'in' on _d_newitemU

6e654d8

In preparation for dlang/dmd#12242.

Geod24 mentioned this pull request Mar 1, 2021

Remove usage of 'in' on _d_newitemU dlang/druntime#3385

Merged

dlang-bot pushed a commit to dlang/druntime that referenced this pull request Mar 1, 2021

Remove usage of 'in' on _d_newitemU

0125219

In preparation for dlang/dmd#12242.

Geod24 force-pushed the preview-in-extern-cpp branch from 2f6e0f6 to c62d09d Compare March 1, 2021 06:21

n8sh pushed a commit to n8sh/druntime that referenced this pull request Mar 3, 2021

Remove usage of 'in' on _d_newitemU

c972a1f

In preparation for dlang/dmd#12242.

Geod24 force-pushed the preview-in-extern-cpp branch from c62d09d to b43665e Compare March 11, 2021 10:05

rymrg pushed a commit to rymrg/druntime that referenced this pull request Mar 15, 2021

Remove usage of 'in' on _d_newitemU

42087a6

In preparation for dlang/dmd#12242.

Geod24 force-pushed the preview-in-extern-cpp branch from b43665e to 5ca6608 Compare March 25, 2021 03:34

thewilsonator mentioned this pull request Apr 12, 2021

Do not recommend un-approved preview switches #12425

Merged

Chigusa0w0 pushed a commit to Chigusa0w0/druntime that referenced this pull request May 2, 2021

Remove usage of 'in' on _d_newitemU

8dae13d

In preparation for dlang/dmd#12242.

dlang-bot added the Review:stalled label Jun 28, 2021

Geod24 force-pushed the preview-in-extern-cpp branch from 5ca6608 to 3e17144 Compare July 11, 2021 08:49

Geod24 force-pushed the preview-in-extern-cpp branch from 122e09b to d1d1be2 Compare June 16, 2022 16:06

dlang-bot removed the Review:stalled label Jun 16, 2022

Geod24 force-pushed the preview-in-extern-cpp branch 5 times, most recently from 3253bd3 to cfab94c Compare June 16, 2022 22:58

Geod24 requested a review from thewilsonator June 16, 2022 23:44

thewilsonator approved these changes Jun 17, 2022

View reviewed changes

thewilsonator added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label Jun 17, 2022

Geod24 mentioned this pull request Jun 17, 2022

Fix 23175 - -preview=in silently adds possible stack memory escape #14219

Closed

dkorpel reviewed Jun 17, 2022

View reviewed changes

Geod24 force-pushed the preview-in-extern-cpp branch from cfab94c to 775b839 Compare June 17, 2022 15:08

ibuclaw approved these changes Jun 18, 2022

View reviewed changes

kinke approved these changes Jun 18, 2022

View reviewed changes

WalterBright reviewed Jun 18, 2022

View reviewed changes

thewilsonator added Merge:auto-merge and removed Merge:72h no objection -> merge The PR will be merged if there are no objections raised. labels Jun 20, 2022

dlang-bot merged commit b3cc517 into dlang:master Jun 20, 2022

Geod24 deleted the preview-in-extern-cpp branch June 20, 2022 12:58

This was referenced Mar 4, 2023

Fix a printf prototype in std.traits test dlang/phobos#8709

Merged

dmd: Deprecate 'in' parameters on non-extern(D,C++) functions #14951

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `-preview=in` only with `extern(D|C++)` #12242

Allow `-preview=in` only with `extern(D|C++)` #12242

Geod24 commented Mar 1, 2021

dlang-bot commented Mar 1, 2021 •

edited

Loading

Geod24 commented Mar 1, 2021

PetarKirov commented Mar 1, 2021

kinke commented Mar 1, 2021 •

edited

Loading

Geod24 commented Mar 2, 2021

PetarKirov commented Mar 2, 2021 •

edited

Loading

Geod24 commented Mar 2, 2021

atilaneves commented Mar 29, 2021

Geod24 commented Jun 16, 2022

thewilsonator commented Jun 17, 2022

dkorpel Jun 17, 2022

Geod24 Jun 17, 2022

Geod24 commented Jun 17, 2022

ibuclaw commented Jun 18, 2022

WalterBright Jun 18, 2022

Geod24 Jun 19, 2022

WalterBright commented Jun 18, 2022

atilaneves commented Jun 20, 2022

Geod24 commented Jun 20, 2022

		@@ -0,0 +1,11 @@
		`-preview=in` can now be used with `extern(C++)`, disabled for other non-D linkage

		The intent of `-preview=in` is to make `in` the go-to storage class for input parameters in D.

Allow -preview=in only with extern(D|C++) #12242

Allow -preview=in only with extern(D|C++) #12242

Conversation

Geod24 commented Mar 1, 2021

dlang-bot commented Mar 1, 2021 • edited Loading

Bugzilla references

Testing this PR locally

Geod24 commented Mar 1, 2021

PetarKirov commented Mar 1, 2021

kinke commented Mar 1, 2021 • edited Loading

Geod24 commented Mar 2, 2021

PetarKirov commented Mar 2, 2021 • edited Loading

Geod24 commented Mar 2, 2021

atilaneves commented Mar 29, 2021

Geod24 commented Jun 16, 2022

thewilsonator commented Jun 17, 2022

dkorpel Jun 17, 2022

Choose a reason for hiding this comment

Geod24 Jun 17, 2022

Choose a reason for hiding this comment

Geod24 commented Jun 17, 2022

ibuclaw commented Jun 18, 2022

WalterBright Jun 18, 2022

Choose a reason for hiding this comment

Geod24 Jun 19, 2022

Choose a reason for hiding this comment

WalterBright commented Jun 18, 2022

atilaneves commented Jun 20, 2022

Geod24 commented Jun 20, 2022

Allow `-preview=in` only with `extern(D|C++)` #12242

Allow `-preview=in` only with `extern(D|C++)` #12242

dlang-bot commented Mar 1, 2021 •

edited

Loading

kinke commented Mar 1, 2021 •

edited

Loading

PetarKirov commented Mar 2, 2021 •

edited

Loading