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

Safe by default v2 #228

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

skyline131313
Copy link

No description provided.

@mdparker
Copy link
Member

Please add your name and contact info to the author field. You can remove Walter's.

DIPs/DIP1044.md Outdated

Interfaces to extern C and C++ functions should always be explicitly marked.

Any unmarked `extern` C and C++ functions will remain `@system`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this includes both body and bodiless extern functions?


## Breaking Changes and Deprecations

This will likely break most code that has not already been annotated with `@safe`,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will likely break some code.

DIPs/DIP1044.md Outdated

Interfaces to extern C and C++ functions should always be explicitly marked.

Any unmarked `extern` C and C++ functions will remain `@system`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is that a lot of those functions are actually D code.

@Bolpat
Copy link
Contributor

Bolpat commented Jun 21, 2022

The DIP does not (explicitly) talk about function pointer and delegate types, especially when those types are used as a function parameter or function return type.
With this DIP, which of the following two is the function declaration

void delegate() getCallback();

equivalent to?

void delegate() @safe getCallback() @safe;

or

void delegate() @system getCallback() @safe;

or some third option I could not think of?

With this DIP, which of the following two is the function declaration

void register(void delegate() callback);

equivalent to?

void register(void delegate() @safe callback) @safe;

or

void register(void delegate() @system callback) @safe;

or some third option I could not think of?

DIPs/DIP1044.md Outdated
@@ -76,9 +76,7 @@ it will no longer link because the mangling will be different. But for `extern (
safety is not part of the mangling and it will compile and link without error. This has always
relied on the user getting it correct.

Interfaces to extern C and C++ functions should always be explicitly marked.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to write such a DIP myself; I was waiting for -preview=dip1000 to become the default. I think that any and all declarations with no body should have explicit @safe/@trusted/@system attributes and that even considering linkage is a mistake.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is that considered a mistake?

You are interfacing with a language that has no notion of any safety guarantees. Ignoring differences between languages and the interfaces between them is a mistake.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because extern(C) does not mean "this is C code", it means "C linkage" and could very well be @safe D code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, a @safe extern(C) function that is not defined in-place is not verified by the compiler – and @safe means that the compiler checked it (up to calls to @trusted). I’m no expert on this, but as far as I understand it’s about mangling. Attributes are not part of the signature of extern(C) functions. extern(C) void f() @safe; could be defined elsewhere as extern(C) void f() @system { … }. I think D needs some solution to this; Maybe I’m naïve, but it could be as easy as generating mock symbols that actually include attributes that lead to linker errors when the attributes don’t match.

At least until 2.096.0, this worked:

import a;
extern(C) void f() @safe;
void main() @safe { f(); }
module a;
extern(C) void f() @system { int* p;  int a;  p = &a; }

If you change extern(C) to extern(D), you get the link error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because extern(C) does not mean "this is C code", it means "C linkage" and could very well be @safe D code.

If it's @safe D code then they can just import it. It doesn't matter if it's C code, or linkage, it is still going by C's rules. Just cause safe D code can have C linkage doesn't mean it should expose a C linkage of safe.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can indeed just import it, but it'll be extern(C) all the same. I don't know what "going by C's rules" means.

If it's D code with C linkage and it's @safe, well, it's @safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Importing does not safeguard you from calling a @safe annotated extern(C) declared function (e.g. from a .di file) whose definition (implementation in a .d file) is annotated @system. That’s because the annotation is not visible for the linker.
I guess the easiest way would be to allow extern(C) @safe function declarations only if they’re also definitions. The proper way would be for D to do something that safeguards you at least in simple and obvious cases.

@ntrel
Copy link
Contributor

ntrel commented Aug 1, 2022

I think a more pragmatic step would be to opt in to @safe per module - @safe module foo;. That is far more flexible than a preview switch and allows mixing use of safe modules with non safe modules in the same compiler invocation. There are lots of awkward issues with changing the default. By opting in per source file those are avoided.

@mdparker
Copy link
Member

I need a name and contact info in the Author field, please. Walter's name goes second if you're reusing content from his original DIP.

@Bolpat
Copy link
Contributor

Bolpat commented Sep 1, 2022

@ntrel, in which regard would @safe module foo; be different from module foo; @safe:?

@ichordev
Copy link
Contributor

ichordev commented Sep 1, 2022

@ntrel, in which regard would @safe module foo; be different from module foo; @safe:?

I'm pretty sure that @safe: won't make struct or class member-functions @safe.

@dkorpel
Copy link
Contributor

dkorpel commented Sep 1, 2022

I'm pretty sure that @safe: won't make struct or class member-functions @safe.

It does! Unlike @nogc nothrow pure, which don't propagate through scopes.

@ntrel
Copy link
Contributor

ntrel commented Sep 1, 2022

@Bolpat @safe: turns off inference of safety for templates, inferred return type functions and function literals. @safe module foo; would just change the default safety but not override inference.

@Bolpat
Copy link
Contributor

Bolpat commented Sep 5, 2022

@ntrel, I used templates and attributes a lot, but never stumbled on this; wow. Probably because I consider attribute: harmful for anything except public and private.

@mdparker
Copy link
Member

@skyline131313 I've emailed you recently about moving this into review. Please get back to me when you're ready to move forward.

@rikkimax
Copy link
Contributor

This will need to show benefit over inference of safety (which it currently does not).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants