Fix Issue 8887 - Disallow passing static arrays by value in extern(C) ... #1215

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
@ghost

ghost commented Oct 24, 2012

I'm unsure about extern(Pascal) and extern(Windows), should those be covered as well?

Also, what does it mean if linkage is LINKdefault?

Fixes Issue 8887 - Disallow passing static arrays by value in extern(…
…C) and extern(C++) functions.

Force -m32 on test-case.
Member

JakobOvrum commented Oct 24, 2012

What if it's linking to C functions implemented in D? Then this becomes a perfectly legitimate thing to do. Same with passing associative array references.

Member

alexrp commented Oct 24, 2012

I don't think I like this change. I realize it's to prevent common errors, but you have to keep in mind that whether a static array (and a static array of a certain size) is passed by reference is completely ABI-dependent.

@ghost

ghost commented Oct 24, 2012

Then why would they be C functions? If a function is defined in D with C linkage, surely you want C apps to be able to use them. C can't pass static arrays by value, so the parameter has to be a pointer. And if only D apps use the function, why not make it have D linkage in the first place?

@ghost

ghost commented Oct 24, 2012

@alexrp: If that's really true then we can close the pull. But I thought you could never pass static arrays by value in C/C++.

Member

alexrp commented Oct 24, 2012

@AndrejMitrovic Sigh, ignore me, I'm spouting nonsense. I somehow managed to conflate this with passing of structs, which is implementation-defined.

As for @JakobOvrum's concern: I think it doesn't make sense in the first place to allow passing static arrays by value in extern (C) functions because they, by definition, follow the C ABI and not the D ABI. If you really must do this, simply embed a static array in a struct - then it gets passed by value even in C.

@ghost

ghost commented Oct 24, 2012

Yeah passing structs is different (and also currently broken but that's a different story).

But I have no idea if Windows or Pascal functions have the same limitations, I don't cover those in this pull. Also I don't cover inout, I'm not sure about its semantics w.r.t C/C++.

Member

alexrp commented Oct 24, 2012

The restriction should apply to those as well. They are all C calling conventions, so they must obey the C rules here.

Member

alexrp commented Oct 24, 2012

(Okay, okay, I know Pascal isn't technically a C calling convention but it was adapted into one at some point by some masochist out there ;)

Owner

WalterBright commented Oct 24, 2012

I'll often use "C" linkage in order to avoid having name mangling for one reason or another.

Member

alexrp commented Oct 24, 2012

@WalterBright #1085 should solve that when it gets merged (after I get around to updating it).

@ghost

ghost commented Oct 24, 2012

@WalterBright: Sounds like Pull 1085 was made for you. :)

Member

JakobOvrum commented Oct 24, 2012

I agree using C linkage just for the mangling is a hack and pragma(mangle, ...) is a significant improvement.

However, pragmas are implementation-specific, so using C linkage is more portable, and I still feel that this change can be detrimental to other code too, I'm just not sure what it might be.

I guess I'll neither vote for or against this one, just voicing some concerns.

@ghost

ghost commented Oct 24, 2012

Well I'm not in a rush to have this merged, we can put it on hold.

Owner

WalterBright commented Oct 25, 2012

This change breaks existing code, requires an awkward workaround for existing uses, and has only a marginal benefit. At this point, I'm opposed to it.

@ghost

ghost commented Oct 25, 2012

Ok, but the existing code has by definition invalid semantics and relies on unimplemented features, and can cause invalid runtime behavior when interfacing with C by introducing a simple typo. Fixing this is not of a marginal benefit, it's in the same boat as the change to disallow passing a extern(D) function pointer to a function expecting an extern(C) pointer.

The code this change would break is code that is already broken.

@ghost ghost closed this Oct 25, 2012

Collaborator

donc commented Oct 25, 2012

The code this change would break is code that is already broken.

It's actually worse than that. It's code that was correct in D1 but is incorrect in D2. (static arrays were not passed by value in D1).

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment