Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed

Conversation

ghost
Copy link

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

…C) and extern(C++) functions.

Force -m32 on test-case.
@JakobOvrum
Copy link
Member

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.

@alexrp
Copy link
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
Copy link
Author

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
Copy link
Author

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++.

@alexrp
Copy link
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
Copy link
Author

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++.

@alexrp
Copy link
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.

@alexrp
Copy link
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 ;)

@WalterBright
Copy link
Member

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

@alexrp
Copy link
Member

alexrp commented Oct 24, 2012

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

@ghost
Copy link
Author

ghost commented Oct 24, 2012

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

@JakobOvrum
Copy link
Member

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
Copy link
Author

ghost commented Oct 24, 2012

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

@WalterBright
Copy link
Member

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
Copy link
Author

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
@donc
Copy link
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 pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants