January 03, 2020
On Friday, 3 January 2020 at 22:23:41 UTC, Guillaume Piolat wrote:
> On Friday, 3 January 2020 at 22:13:28 UTC, Steven Schveighoffer wrote:
>>
>> I'd probably have to adjust a few things, and then it will build. The benefit is huge, because then someone doesn't have to choose between memory safety and iopipe speed. If the compiler complained about this when I was developing it, I'd fix the problems as they happened.
>>
>
> Can anyone produce ONE example of a memory corruption code they write that was found with use of marking something @safe?

Not sure, if this fits here. But if I add ´´´@safe:´´´ at the beginning of the module, the library https://code.dlang.org/packages/vebtree does not compile any more.
January 03, 2020
I strongly oppose such a move.

I am in all likelyhood going to mark every line of code with @trusted which is a bold face lie, as I appear to be the tiny minority of people who actually likes C and uses pointers as the most reliable tool in my tool box.

You will be increasing verbosity, and promoting lying code; because this is just how I will behave, because I didn't and will not ever be going to looking towards rust as the C replacement. Yet C needs to be replaced and there isn't exactly a slew of options. So why are you taking pages from rust?

I'm here because you offer compile time abstractions on simple structs that are not the modern c++ mess, as modern cpus break more and more assumptions C made, those abstracts will be nessery with pointer math, casting and other "discouraged" code to get the most speed. And people looking for the exact same thing I'm looking for with that same style will lie to the compiler in the exact same way.

Theres needs a non-verbose, non-lying way to opt-out, and probably shouldn't be opt-in in the first place.

I would suggest module scope default behaviors.
January 03, 2020
On Fri, Jan 03, 2020 at 10:23:41PM +0000, Guillaume Piolat via Digitalmars-d wrote:
> On Friday, 3 January 2020 at 22:13:28 UTC, Steven Schveighoffer wrote:
> > 
> > I'd probably have to adjust a few things, and then it will build. The benefit is huge, because then someone doesn't have to choose between memory safety and iopipe speed. If the compiler complained about this when I was developing it, I'd fix the problems as they happened.
> > 
> 
> Can anyone produce ONE example of a memory corruption code they write that was found with use of marking something @safe?

Ran into this a long time ago:

	struct ResourceA { /* ... */ }
	struct ResourceB { /* ... */ }
	// ...

	ResourceA getResourceA() @safe { return ResourceA(); }
	ResourceB getResourceB() @safe { return ResourceB(); }
	void freeResourceA(ResourceA*) @safe { /* ... */ }
	void freeResourceB(ResourceB*) @safe { /* ... */ }

	struct Container {
		ResourceA resA;
		ResourceB resB;
		// ...

		void delegate()[] dtors;

		this(int blah) /*@safe*/ {
			resA = getResourceA();
			dtors ~= { freeResourceA(&resA); };

			resB = getResourceB();
			dtors ~= { freeResourceA(&resA); };

			// ...
		}
		~this() {
			// Cleanup resources
			foreach (dtor; dtors)
				dtor();
		}
	}

The idea is that each allocated resource must be cleaned up when the container goes out of scope. To prevent cleanup code from going out of sync with allocation code, it's appended to a list of dtors immediately after allocation (sorta like a manual version of scope(exit) that works over an object's lifetime).

Exercise for the reader: spot the bug.

So, with @safe commented out in Container.this() as shown above, this compiles fine with 'dmd -dip1000', even though DIP1000 ostensibly is supposed to prevent precisely this sort of memory corruption.

Uncomment the @safe, and recompile, the compiler identifies the problem:

	test.d(17): Error: reference to local this assigned to non-scope this.dtors in @safe code
	test.d(20): Error: reference to local this assigned to non-scope this.dtors in @safe code

The reason is that DIP1000 only performs these checks in @safe code (and for good reasons -- in @system code presumably you know what you're doing, and should be able to do apparently unsafe things, the assumption being that you've taken measures to prevent problems that the compiler cannot mechanically verify).


T

-- 
What's a "hot crossed bun"? An angry rabbit.
January 03, 2020
On 1/3/2020 12:02 PM, bachmeier wrote:
>> What it will do is necessitate labeling code with @system that were always @system already.
> I'll send you my code and hopefully you can make all the necessary changes in a timely manner. Frankly, I'm shocked that you all of a sudden think major breaking changes in the language are no big deal.

No refactoring, changing data structures, anything like that, will be required. It's just adding @system where the compiler indicates. Most of the job will be simply adding:

   @system:

to the beginning of the file. Something similar to this was done to the DMD backend after it was converted to D - a bunch of `nothrow:` lines were inserted at the beginning of the modules. Didn't take more than a few minutes.


> What really has me puzzled is that this is a change with such a limited benefit. D already has @safe. If you want to make safe by default an option, add a -safe flag to the compiler

This feature is enabled with -preview=safedefault. You'll have years to it which to pick a time to annotate with @system as required.


> and the problem is solved with no code breakage and no confusion, and you can appeal to the tiny sliver of programmers Rust was created for and that will never be interested in D anyway. Breaking changes can be good, but careful thought needs to be used to minimize the cost of those changes.

Rust has changed the conversation. Major languages are all already safe by default, or are moving that direction. We don't have a choice.





January 03, 2020
On 1/3/2020 5:57 AM, Arine wrote:
> Then this would be better served as an opt in feature.

It will be opt-in for a time, that's why it will be enabled with -preview=safedefault


> This is going to be a big breaking change, and if you are going to do the same thing with `nothrow`, that's way too much breakage for very little benefit just to follow a trend.

There are compelling reasons for nothrow by default. I suggest deferring this discussion until the DIP is put forward for review.


> Especially if steps aren't going to be taken to ensure it is easy to maintain backwards compatibility. As someone else mentions
> 
> @system:
> 
> does not give the same behavior and will still break code.

It's still easy to deal with.


> There's probably already an issue filed for it. It comes up often. I don't have the time right now to search through tens of thousands of unmanaged issues for you. I already gave an example in my preview post.

Spending time guessing and speculating on what it might be does a disservice to those who do spend the time filing issues, on which I and others do spend effort to address.

> tens of thousands of unmanaged issues

This is why there are bugzilla keywords for categorizing issues, like `safe` for all safety related issues:

https://issues.dlang.org/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&keywords=safe&keywords_type=allwords&list_id=229608&query_format=advanced
January 03, 2020
On 1/3/2020 7:29 AM, Steven Schveighoffer wrote:
> You want as LITTLE as possible to be marked @system, not a blanket marking.

Sure, but if you want to do as little work as possible, blanket marking will do most of the job.
January 03, 2020
On 1/3/2020 5:03 AM, Guillaume Piolat wrote:
> I think what will be useful is accepting the change as -preview, let us build with it and then see how many actual bugs it catched.

That's what the DIP says!


>> That was indeed the idea behind unsafety as the default. But the programming world has changed. We don't have a choice.
> Well yes this sounds like the sort of decision that can avoid loss of millions in the far future. It's just a bit annoying, no biggie.

Yes. Although it will break a lot of code, the fix (annotate with @system where the compiler indicates) is straightforward.

January 03, 2020
On Friday, 3 January 2020 at 22:59:53 UTC, Walter Bright wrote:
> On 1/3/2020 5:57 AM, Arine wrote:
>> Then this would be better served as an opt in feature.
>
> It will be opt-in for a time, that's why it will be enabled with -preview=safedefault

Yes please.

>> Especially if steps aren't going to be taken to ensure it is easy to maintain backwards compatibility. As someone else mentions
>> 
>> @system:
>> 
>> does not give the same behavior and will still break code.
>
> It's still easy to deal with.

I expect the change of default to be a _lot_ of work to deal with at Weka. I think it will not be easy at all, because there is a lot of templated code and code checking whether something compiles or not (`__traits(compiles)`). Every compiler update changes what compiles and does not compiles, and it is almost always very hard to track down how that changes the behavior of Weka's code. (If something doesn't compile, the error happens in a template that cannot be instantiated because some template 5 layers down is not instantiatable for example. The error won't say that of course...)
With the @safe change, I'm expecting 100s of templates that will no longer compile or change behavior, and for very few of the compile errors will it be obvious what is going on. As said before, `@system:` is not going to help.

I have no opinion on whether this is a good change or not. I just very much would like to get the message across that this change will be very disruptive and so please provide a long deprecation time (e.g. something like 2 years).

-Johan

January 03, 2020
On Friday, 3 January 2020 at 22:47:05 UTC, H. S. Teoh wrote:
> On Fri, Jan 03, 2020 at 10:23:41PM +0000, Guillaume Piolat via Digitalmars-d wrote:
>> On Friday, 3 January 2020 at 22:13:28 UTC, Steven Schveighoffer wrote:
>> > 
>> > I'd probably have to adjust a few things, and then it will build. The benefit is huge, because then someone doesn't have to choose between memory safety and iopipe speed. If the compiler complained about this when I was developing it, I'd fix the problems as they happened.
>> > 
>> 
>> Can anyone produce ONE example of a memory corruption code they write that was found with use of marking something @safe?
>
> Ran into this a long time ago:
>
> 	struct ResourceA { /* ... */ }
> 	struct ResourceB { /* ... */ }
> 	// ...
>
> 	ResourceA getResourceA() @safe { return ResourceA(); }
> 	ResourceB getResourceB() @safe { return ResourceB(); }
> 	void freeResourceA(ResourceA*) @safe { /* ... */ }
> 	void freeResourceB(ResourceB*) @safe { /* ... */ }
>
> 	struct Container {
> 		ResourceA resA;
> 		ResourceB resB;
> 		// ...
>
> 		void delegate()[] dtors;
>
> 		this(int blah) /*@safe*/ {
> 			resA = getResourceA();
> 			dtors ~= { freeResourceA(&resA); };
>
> 			resB = getResourceB();
> 			dtors ~= { freeResourceA(&resA); };
>
> 			// ...
> 		}
> 		~this() {
> 			// Cleanup resources
> 			foreach (dtor; dtors)
> 				dtor();
> 		}
> 	}
>
> The idea is that each allocated resource must be cleaned up when the container goes out of scope. To prevent cleanup code from going out of sync with allocation code, it's appended to a list of dtors immediately after allocation (sorta like a manual version of scope(exit) that works over an object's lifetime).
>


(I catched your bug while reading (&resA is dupe), but would make similar bugs of course)

Just a note that people have sent me 3 such bugs they had and that @safe indeed catch (sometimes in association with -dip1000)


------- sample 2 ---------

    import std.stdio;
    void main()
    {
        int[3] q;
        oh(cast(int[4]*)(cast(void*)q));
    }

    void oh(int[4]* ar)
    {
        writeln(*ar);
    }

------- sample 3 ----------

    import std;

    struct User
    {
        string username;
        ubyte[] password;
    }

    ubyte[20] hashPassword(const(char)[] password) {
         import std.digest.sha;

         return sha1Of(password);
    }

    User admin;

    void registerAdmin() {
        auto hash = hashPassword("hello");

        admin.username = "bob";
        admin.password = hash;
    }

    void printAdmin() {
        writeln(admin);
    }

    void main() {
        registerAdmin();
        printAdmin();
    }


So we can get an idea of the kind of things it will catch.
January 03, 2020
On 1/3/2020 3:30 PM, Johan wrote:
> I expect the change of default to be a _lot_ of work to deal with at Weka. I think it will not be easy at all, because there is a lot of templated code and code checking whether something compiles or not (`__traits(compiles)`). Every compiler update changes what compiles and does not compiles, and it is almost always very hard to track down how that changes the behavior of Weka's code. (If something doesn't compile, the error happens in a template that cannot be instantiated because some template 5 layers down is not instantiatable for example. The error won't say that of course...)
> With the @safe change, I'm expecting 100s of templates that will no longer compile or change behavior, and for very few of the compile errors will it be obvious what is going on. As said before, `@system:` is not going to help.

Templates without an explicit @safe/@trusted/@system will still get their safety attributes inferred as before.


> I have no opinion on whether this is a good change or not. I just very much would like to get the message across that this change will be very disruptive and so please provide a long deprecation time (e.g. something like 2 years).

That's probably a good idea. Keep in mind that even when it is no longer a -preview feature, there'll be a -revert switch there for a long time to continue to use the old behavior.

I am interested to hear about your experience with this on Weka's codebase, and am open to your advice on the timing.
2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18