On 6/17/2021 8:59 AM, H. S. Teoh wrote:
> What are the actual advantages of code being marked pure?  I'm all for
> generalizing pure, but does it bring enough actual benefits to be worth
> the effort?
> 
> I'm not talking about theoretical benefits, but actual benefits that the
> compiler can actually make use of to emit better code.  I used to be a
> big fan of pure, but in practice I've found that it doesn't make *that*
> much of a difference in terms of codegen.  Maybe I'm missing something,
> in which case I'd love to be enlightened.

You're right it doesn't make that much difference in code quality. What it *does* provide is:

1. makes it easy to reason about

2. makes unit testing easy

Just think about trying to unit test a function that side-loads various globals.

On Thu, Jun 17, 2021 at 11:25:44AM -0700, Walter Bright via Digitalmars-d wrote:
> On 6/17/2021 8:59 AM, H. S. Teoh wrote:
> > What are the actual advantages of code being marked pure?  I'm all for generalizing pure, but does it bring enough actual benefits to be worth the effort?
> > 
> > I'm not talking about theoretical benefits, but actual benefits that the compiler can actually make use of to emit better code.  I used to be a big fan of pure, but in practice I've found that it doesn't make *that* much of a difference in terms of codegen.  Maybe I'm missing something, in which case I'd love to be enlightened.
> 
> You're right it doesn't make that much difference in code quality. What it *does* provide is:
> 
> 1. makes it easy to reason about
> 
> 2. makes unit testing easy
> 
> Just think about trying to unit test a function that side-loads various globals.

I guess, as a side-effect of the way I usually code, which is to avoid globals unless I absolutely cannot get around it, I'm not really seeing the benefits of adding `pure` to my function declarations. :-D  Almost all of my code is already pure anyway, it's just more work to tag them with `pure`.  So if it doesn't bring significant additional benefits, I'm not seeing it as pulling its own weight.

On a tangential note, when it comes to unittests, you're right that globals make things hard to test.  The same also applies for code that modify the state of the environment, e.g., the filesystem. In Phobos there used to be (still are?) unittests that create temporary files, which makes them hard to parallelize and occasionally prone to random unrelated breakage.  In my own code, I sometimes resort to templatizing filesystem-related functions/types so that I can test the code using a mock-up filesystem instead of the real one, e.g.:

	// Original code: hard to test without risking unwanted
	// interactions with the OS environment
	auto manipulateFile(File input) {
		...
		auto data = input.rawRead(...);
		...
	}

	// More testable code
	auto manipulateFile(File = std.stdio.File)(File input) {
		...
		auto data = input.rawRead(...);
		...
	}

	unittest {
		struct MockupFile {
			... // mockup file contents here
			void[] rawRead(...) { ... }
		}

		// Look, ma!  Test a filesystem-related function without
		// touching the filesystem!
		assert(manipulateFile(MockupFile(...)) == ...);
	}


T

-- 
Life is complex. It consists of real and imaginary parts. -- YHL

Am 16.06.2021 um 23:22 schrieb Walter Bright:
> On 6/16/2021 6:09 AM, Sönke Ludwig wrote:
>> There are 800 of these in vibe.d alone.
> 
> That is concerning. But it isn't necessarily cause for redesigning @trusted. For example, I removed (in aggregate) a great deal of unsafe allocation code from the backend simply by moving all that code into one resizable array abstraction.
> 
> Piece by piece, I've been removing the unsafe code from the backend. There really should be very, very little of it.

Many of them are external functions that are `@system` when they shouldn't have to be:
									  - `() @trusted { return typeid(string).getHash(&ln); }()));`
- `() @trusted { return allocatorObject(GCAllocator.instance); } ();`
- `() @trusted { GC.addRange(mem.ptr, ElemSlotSize); } ()`
- `() @trusted { return sanitize(ustr); } ()`
- `() @trusted { return logicalProcessorCount(); } ()`
- ...

It could be that nowadays a number of those has been made `@safe` already, I'd have to check one-by-one.

Then there are OS/runtime functions that are not `@safe`, but need to be called from a `@safe` context:

- `() @trusted { return mkstemps(templ.ptr, cast(int)suffix.length); } ();`
- ```
    @trusted {
        scope (failure) assert(false);
        return CreateFileW(...);
    } ();
    ```
- ...

There is also quite some manual memory management that requires `@trusted`. Once we are there with ownership (and ten compiler versions ahead) those can be replaced by some kind custom reference type.

Then there are some shortcut references as pointers that are necessary because `ref` can't be used for local symbols (lifetime analysis could solve this, too):

- `auto slot = () @trusted { return &m_core.m_handles[h]; } ();`

There are surely some places that can be refactored to push `@trusted` further down, but right now most of them can't in a meaningful way.

On 6/17/21 12:21 PM, Paul Backus wrote:
> On Thursday, 17 June 2021 at 14:30:58 UTC, Steven Schveighoffer wrote:
>> On 6/16/21 9:07 PM, Paul Backus wrote:
>>>
>>> It's impossible to guarantee, at the language level, that @safe code can never require manual review. The programmer is allowed to use any and all knowledge at their disposal to verify the memory safety of @trusted (or in your proposal, @system-block) code, including knowledge about @safe code.
>>
>> The goal is to guarantee that *as long as* your @trusted functions and blocks have a @safe interface, then @safe code does not need to be checked. When I say "not require review" I mean "I have checked all the @trusted code, and it has a sound @safe interface, so all @safe code that may call it have no need for review." We will never have a marking that is language-guaranteed to not require review.
>>
>> To put it another way, as long as you aren't using @trusted escapes that leak implementation details, your @safe code shouldn't need a review. The problem is that trusted lambdas are not only the norm, it's actually required, due to template inference.
>>
>> Right now, a @safe function can only be "assumed safe" as long as there are no @trusted blocks in it. Once there is one trusted block, then you have to review the whole function. The same thing goes for data invariants (though that spreads to the whole module instead).
>>
>> Not having to review code for memory safety is supposed to be the major point of @safe.
> 
> Consider the following example:
> 
> ```d
> size_t favoriteNumber() @safe { return 42; }
> 
> int favoriteElement(ref int[50] array) @trusted
> {
>      // This is memory safe because we know favoriteNumber returns 42
>      return array.ptr[favoriteNumber()];
> }
> ```
> 
> `favoriteElement` has a safe interface. There is no argument you can pass to it from `@safe` code that can possibly result in memory corruption.
> 
> However, if you change `favoriteNumber` to return something different (for example, 69), this may no longer be the case. So changes to `favoriteNumber`--a `@safe` function with no `@trusted` escapes--must still be manually reviewed to ensure that memory safety is maintained.

But that's a different kind of problem. If favoriteNumber is allowed to return anything above 49, then favoriteElement is invalid.

If favoriteNumber is *required* by spec to return 42, then it needs to be reviewed to ensure that it does that. And then the @safe function does not need to be reviewed for *memory* problems, just that it's written to spec.

> 
> There is no language change you can make (short of removing `@trusted` entirely) that will prevent this situation from arising.

Sure, code reviews on @safe code still need to happen to ensure they are correct. But code reviews for *memory safety errors* which are *really hard* to do right, should not be required.

It's similar to unit-tests. Instead of testing the whole project at once, you test one thing, and if all the little things are correct, the combination of things is valid.

-Steve

On 17.06.21 20:46, Ola Fosheim Grøstad wrote:
> What about it isn't safe? It is provably safe? Meaning, I can do a formal verification of it as being safe!?

`offset` is an input to `get` (via `this`). `offset` is an int, so all possible values (int.min through int.max) are considered "safe values". `get` exhibits undefined behavior when `offset` is greater than 1. A function that can exhibit undefined behavior when called with only safe values cannot be @trusted.

> If this isn't safe then it becomes impossible to write @safe wrappers for C data structures.

As I wrote, DIP 1035 addresses this.