dip1000 and preview in combine to cause extra safety errors

[John Colvin]

On Monday, 13 June 2022 at 11:14:36 UTC, deadalnix wrote:
> On Wednesday, 8 June 2022 at 17:50:18 UTC, John Colvin wrote:
>> The compiler is going “you told me `foo` doesn’t leak 
>> references to the string passed to it, I believe you. Based on 
>> that, this temporary array is safe to put on the stack”. I 
>> think it’s reasonable for the compiler to lean on `scope` like 
>> this.
>>
>> The problem is `foo` and whether the compiler should somehow 
>> prevent the inconsistency between the signature and 
>> implementation. Obviously the answer is “yes, ideally”, but in 
>> practice with @safe, @system, dip1000, @live and so on it’s 
>> all a mess.
>
> So I gave it some time, and I think I am now convinced that 
> doing this optimization is simply not a good idea.
>
> If the value stays on stack - which is all that DIP1000 can 
> check for anyways, then modern backend can track it. LLVM for 
> instance, will annotate function parameter to indicate if they 
> escape or not and do so recursively through the callgraph.
>
> LDC is already able to do stack promotion when escape analysis 
> proves something doesn't escape.
>
> This is WAY preferable because:
>  - It works regardless of annotations from the dev.
>  - It is always correct, it will not fubar existing @system 
> code.
>  - Inlining is likely to uncover more opportunity to do this, 
> there is no point doing it before.
>
> Doing this type of optimization to explicitly free elements on 
> heap is worth it. But DIP1000 doesn't allow to track this 
> reliably.

Without expressing an opinion either way, I want to note that 
this has implications for `@nogc`. If you remove `scope` from the 
parameter of `foo`, the compiler won't let `main` be `@nogc` due 
to the slice literal allocation.

     string foo(scope string s) @nogc {
         return s;
     }

     void main() @nogc {
         foo(['a']);
     }