Feedback Thread: DIP 1028--Make @safe the Default--Final Review

[Jonathan M Davis]

On Saturday, April 11, 2020 1:30:26 AM MDT Walter Bright via Digitalmars-d 
wrote:
> On 3/25/2020 9:57 PM, Jonathan M Davis wrote:
> > There's also the question of declaration vs definition for functions
> > which aren't extern(D). It makes no sense for a function declaration
> > which is not extern(D) to be treated as @safe by default, because the
> > compiler can't guarantee its @safety. However, I don't think that it
> > would be a problem for non-extern(D) function definitions, because then
> > the compiler _can_ check their @safety. Either way, the DIP should be
> > explicit about what happens with declarations and definitions which are
> > not extern(D) - regardless of whether declarations and definitions are
> > treated differently.
> >
> > I'd also argue that @safe should not be allowed on function declarations
> > which are not extern(D), because the compiler can't guarantee their
> > @safety (meaning that if an explicit @safety attribute is supplied, it
> > must either be @system or @trusted), and allowing @safe on
> > non-extern(D) declarations makes it much harder for programmers to grep
> > for @trusted code that could be the source of @safety problems. In
> > practice, right now, I'm unaware of anyone marking extern(C)
> > declarations as @safe, but if someone did, it could be very hard to
> > track down the problem if it were hiding a memory safety bug, and given
> > the ease of marking all declarations and definitions in a module with
> > @safe via @safe: or @safe {}, it wouldn't surprise me if some code
> > bases are accidentally marking extern(C) declarations with @safe right
> > now and thus hiding memory safety issues. However, allowing @safe on
> > non-extern(D) function _definitions_ should be fine, since then the
> > compiler actually is verifying the code. Regardless, much as I think
> > that the DIP _should_ make it illegal to mark non-extern(D)
> > declarations as @safe, that would arguably be an improvement over what
> > the DIP currently does rather than being required for the DIP to
> > actually make sense or be acceptable.

Mike clearly doesn't want responses here, but I don't know how I'm supposed
to put a response in the discussion thread when what I'm responding to is in
the feedback thread.

> On the other hand,
>
> 1. it's a special case inconsistency, which has its own costs and
> confusion.

What this DIP proposes will cause even more confusion, and the "special
case" here should be incredibly clear. The compiler cannot treat _anything_
as @safe unless it can mechanically verify that it's @safe, or the
programmer has marked it as @trusted, indicating that _they_ have verified
it. And thus, as the compiler cannot determine that a non-extern(D)
declaration is memory safe, it cannot treat it as @safe. To do anything else
actually makes @safe harder to understand, not easier.

It also makes it far, far more likely that extern(C) declarations will be
incorrectly marked - especially since right now, leaving them unmarked
results in them being correctly marked as @system, whereas after the change,
properly written C bindings would suddenly be treated as @safe even though
they had not been verified by the programmer to be @safe. So, this DIP
_will_ introduce bugs into existing code, and it will make writing new
extern(C) declarations that much more error-prone.

> 2. the compiler cannot verify any extern declarations as being safe, even
> D ones. It's always going to be up to the user to annotate them
> correctly.

Sure, but with the status quo, the compiler isn't lying about what it's
verified, and you won't accidentally end up with code being treated as @safe
just because an attribute was missed. @safe is about mechanical verification
of memory safety, and extern(C) declarations cannot be mechanically
verified, so it makes no sense for the compiler to treat them as @safe.

> 3. the extern(C) specifies an ABI, it doesn't say anything about how the
> function is implemented, or even which language it is implemented in. A
> pretty big chunk of the dmd implementation (80-90%?) is extern(C++)

Sure, and because the compiler hasn't verified it for memory safety, it's
_completely_ inappropriate for it to claim that it has by treating it as
@safe.

> 4. it's trivial to mark a block of C function declarations with @system
> and trivial to audit it. I've done it already to a bunch of druntime
> headers
>
> 5. D's separate compilation model relies on extern declarations where
> source is not available and safety cannot be machine checked. It's
> inherent

Sure, but extern(C) declarations aren't always in separate modules where it
would make sense to just slap @system: at the top, and you're basically
proposing that @trusted be reversed for extern(C) declarations. Right now,
they're not assumed to be @safe unless the programmer has gone to the effort
of indicating that they've verified them, whereas with the DIP, they will
assumed to have been verified by the programmer unless they've been marked
with @system to indicate that the programmer hasn't verified them or that
they've been verified to _not_ be memory safe. That's completely backwards
from how @trusted and @safety in general is supposed to work. It's also much
more error-prone. You're essentially asking the programmer to explicitly
mark code where there might be bugs rather than having them mark where
they've verified that there aren't.

> 6. We're just talking about the default. The whole point of @safe being
> the default is that it is far and away the most common case, even for C
> functions.

Yes, and you're making the default error-prone and confusing for no benefit
to the programmer. These functions have _not_ been verified by the compiler,
so having the compiler mark them as @safe is a lie, and it makes the
programmer's job harder. They then have to worry about tracking down
extern(C) declarations that haven't been marked and whether there are bugs
in the code, because a programmer failed to mark an extern(C) declaration
explicitly as @system (and it could easily be that that declaration had been
written prior to this DIP coming into effect, meaning that it had been
correct at the time it was written but isn't anymore).

Regardless of how likely it is that a particular C function is actually
memory safe, unless the compiler can verify that, having it treat it like it
is just means that that function is not necessarily being checked by anyone,
whereas with the status quo, the programmer will get errors if they try to
use it in @safe code, forcing them to realize that it hasn't been marked as
@trusted and that it either should be marked as @trusted, or it needs to be
treated as @system.

It would make far, far more sense to require that all non-extern(D)
declarations be explicitly marked with @system or @trusted than to have the
compiler blindly treat them as @safe.

> 7. A function having different attributes depending on whether or not a
> body is present is surprising behavior

Is it really? It should be pretty clear that the compiler can't verify a
function declaration for memory safety (even in the case of extern(D)
function declarations, the compiler isn't verifying anything when compiling
the declaration - it just knows that the body was checked). I would think
that having the compiler treat code that it can't verify (and can't know was
verified) for memory safety as @safe would be far more surprising behavior.

And if you think that having function declarations being treated differently
than function definitions would be confusing, then simply treating all
non-extern(D) functions as @system by default shouldn't be confusing. The
rule for that is _really_ simple, and unlike what the DIP proposes, it
wouldn't result in invisible problems. It's less user-friendly for anyone
writing extern(C) functions in D, but at least, it wouldn't result in the
compiler lying about what it's verified for memory safety, and you wouldn't
even potentially be introducing confusion about declarations and definitions
being treated differently.

Alternatively, you could just always require that in any cases where the
compiler can't even attempt to verify code for @safety, an @safety attribute
must be explicitly provided. Arguably, that's what should be happening with
extern(C) declarations anyway, and it's essentially what you're claiming is
best practice in the DIP. Why not just enforce that with the compiler? It
would avoid having the compiler claim that code is @safe when it hasn't
verified diddly-squat, and it would avoid extern(C) declarations being
accidentally treated as @safe.

> 8. annotating "extern(C) void free(void*);" as @safe doesn't make it safe,
> either, again relying on the user

IMHO, marking such a function declaration with @safe shouldn't even be
legal, because the compiler has not verified it for memory safety. @trusted
is the appropriate attribute for that. And while the programmer could also
incorrectly mark the function declaration with @trusted, at least as long as
you can't mark anything with @safe which isn't mechanically verified for
@safety by the compiler, you know that any memory safety bugs you run into
are caused by @trusted code (or @system code that @trusted code calls) and
that that's where you need to look for them. As long as non-extern(D)
function declarations can be either implicitly or explicitly marked with
@safe, you can't find memory safety bugs just by looking for @trusted code,
and wasn't part the whole point of @trusted to make it so that you could
segregate all code that hasn't actually been verified for memory safety by
the compiler so that the programmer can easily find the code that could be
causing memory safety bugs?

We should be making it illegal to mark anything as @safe which the compiler
has not actually verified rather than making the compiler treat code that it
hasn't verified as @safe just because it can't check it.

> 9. what do we do with "nothrow" by default? Say this doesn't apply to
> extern(C++) functions? Is anyone going to remember all these special
> cases?

nothrow by default would be a disaster even for extern(D) given that
exceptions are by far the best way in general to deal with error conditions,
and trying to disable them by default makes them much harder to use. So, I
_really_ hope that you're not seriously considering making nothrow the
default.

Regardless, treating non-extern(D) declarations as nothrow by default
presents many of the same problems as treating them as @safe by default and
makes no sense for the same reasons. The compiler can't verify that they're
nothrow, so it should _not_ be treating them as nothrow. That should be
simple for anyone to remember and understand. And regardless, compiler error
messages should make it crystal clear if you try to call an extern(C++)
function from a nothrow function, just like they should make it crystal
clear if you try to call an @system function from an @safe function.

You seem to be the only person in any of the discussion for @safe by default
who thinks that it makes any sense for extern(C) function declarations to be
treated as @safe by default, and the only logic you seem to have to support
is

1. You think that it would be a confusing special case if non-extern(D)
function declarations were treated differently from function definitions (as
would be the case if non-extern(D) function declarations were @system by
default or required explicit @safety attributes), and you think that it
would be a confusing special case to treat non-extern(D) functions
differently from extern(D) functions (as would be the case if non-extern(D)
declarations and definitions were @system by default or required explicit
@safety attributes).

2. You seem to think that just because it's ultimately up to the programmer
to not make mistakes with extern(C) declarations, the compiler shouldn't
help them at all (especially if that help requires introducing a special
case).

It can easily be argued that treating extern(C) function declarations as
@safe instead of @system is more confusing and that the fact that the
compiler is claiming that code has been verified for memory safety when it
hasn't been is _far_ more confusing. Regardless, I don't think that you can
possibly argue that what you're proposing isn't more error-prone than the
status quo. And pretty much everyone else in the discussion on this seems to
think that because it's trivial for the compiler to see that an extern(C)
declaration has not been verified for @safety, the compiler should use that
information to help the programmer, not ignore it. Treating extern(C)
declarations as @safe _will_ introduce bugs rather than catch them, and it
doesn't make the programmers life any easier.

_Please_ reconsider your position on this. If this DIP goes through, then it
will definitely not be the case in D that @safe code has been verified by
the compiler for memory safety. It won't even be the case that it's either
been verified by the compiler or the programmer. You're about to drive a
truck through @safety just so that you can make @safe the default in places
where in does make sense. There's no need to make it the default for
non-extern(D) declarations in order to make it the default for function
definitions, and you seem to be the only person here who thinks that it
would make sense to do so. As this DIP stands, I pray to God that Atila
rejects this DIP if you won't. And at least from the perspective of anyone
actually using the compiler rather than writing it, it would be _so_ simple
and straightforward to just treat extern(C) declarations as @system - and it
would avoid bugs in the process at no cost to the programmer, whereas this
DIP will introduce and hide bugs. And if it really is best practice to mark
all non-extern(D) declarations explicitly with either @system or @trusted,
then why not just require it? It would result in fewer bugs without making
anything confusing.

If this DIP goes through as-is, we will be stuck explaining to D programmers
for years to come why they have to be wary of the compiler treating
extern(C) function declarations incorrectly and introducing invisible memory
safety bugs into your program if you're not very careful. The compiler will
actively be making dealing with extern(C) harder and more error-prone - and
require more explanation to programmers learning D. On the other hand, if
non-extern(D) declarations are @system by default or require explicit
@safety attributes, then it's _simple_ to explain to people that that means
that they need to be verifying the bindings for @safety and use @trusted
appropriately - or that they need to just mark it them with @system and
leave it up to whoever is using them. The only memory bugs introduced at
that point will be because @trusted was used incorrectly rather than because
an extern(C) declaration was incorrectly treated as @safe, because the
programmer missed it.

- Jonathan M Davis