On Wed, Jul 24, 2019 at 07:06:07AM +0000, aliak via Digitalmars-d wrote:
> On Tuesday, 23 July 2019 at 17:17:25 UTC, H. S. Teoh wrote:
[...]
> > Isn't this precisely the sort of problem dip25 / dip1000 are supposed to catch?  Are you compiling with -dip25 -dip1000?  Are DIP25/DIP1000 (or their current implementations) not catching this particular case for some reason?
[...]
> Not compiling with those no.

You probably should, if you're relying on @safe in any significant way.


> Is @safe supposed to solve all memory corruption or is it only if dip1000 and dip25 is on. So default @safe is not @safe?

@safe is *intended* to prevent memory corruption.  But there are some loopholes in the current implementation, which are addressed by DIP1000 and DIP25.  AIUI, the intention is that -dip25 and -dip1000 will eventually be the default behaviour; we're just in the transition period right now because they could potentially break existing code (as in, they will cause compilation errors to otherwise working code).  So in the meantime, if your code doesn't break with -dip25 -dip1000, you should probably just compile with them by default in order to get the benefits of a better @safe implementation.


T

-- 
"Holy war is an oxymoron." -- Lazarus Long

On Wednesday, 24 July 2019 at 13:40:10 UTC, Timon Gehr wrote:
> On 24.07.19 09:18, aliak wrote:
>> On Tuesday, 23 July 2019 at 15:40:42 UTC, Timon Gehr wrote:
>>> On 23.07.19 15:51, aliak wrote:
>>>>
>>>> (Side note: the specification for return ref - https://dlang.org/spec/function.html#return-ref-parameters - says that "inout ref parameters imply the return attribute.", but I got corruption with "ref inout(T) front() inout {...}". Is that a bug with inout?)
>>>
>>> I think in this case the specification is at fault. It's perfectly fine (and useful) to have an `inout` function that returns `void`.
>> 
>> Void would not be a ref so return would not be inferred (according to the spec)?
>
> No? The spec says that inout ref parameters imply the return attribute.

Ah right! Parameters.

On Wednesday, 24 July 2019 at 08:46:18 UTC, aliak wrote:
>> There is one issue with dub that I know of. When you use dip1000 and use dependencies that don't, things in the dependency might mangle differently between the lib build and your project build if they have an extra attribute inferred.
>
> These are the kinds of things I'm afraid of actually. Is that a dub issue though?

It is not a bug. It is just a result of compiler flags, inferred attributes, templates and separate compilation.

The problem comes about because a template from a dependency - which is compiled without dip1000/25 - is instantiated in your project - which is compiled with dip1000/25 - and then that template calls code from the dependency, where different attributes are inferred in each compilation. (an attribute changes the mangling).

The easiest solution is to use the sourceLibrary target type for the dependency, if the dependency is dip1000/25 compatible. Which it really should be. Unless it is doing complex stuff, in which case, it really should be.

I don't know. I suppose dub needs to propagate dip flags to all dependencies. But there are downsides to that as well.

> Also if the depdendency is not dip1000/dip25 compatible (Which i assume - maybe incorreclty - that most are not) then what? Have you encountered much of that?

If you aren't using templates from the library that also call into the library, there is no issue. Otherwise, either pull-requests to the library or no dip1000/25.

I guess we just need to get dip1000/25 on by default as quickly as possible. But the problem is going to surface again when there are other feature flags that (in)directly influence mangling.

On Wednesday, 24 July 2019 at 16:23:47 UTC, H. S. Teoh wrote:
> On Wed, Jul 24, 2019 at 07:06:07AM +0000, aliak via Digitalmars-d wrote:
>> On Tuesday, 23 July 2019 at 17:17:25 UTC, H. S. Teoh wrote:
> [...]
>> > Isn't this precisely the sort of problem dip25 / dip1000 are supposed to catch?  Are you compiling with -dip25 -dip1000?  Are DIP25/DIP1000 (or their current implementations) not catching this particular case for some reason?
> [...]
>> Not compiling with those no.
>
> You probably should, if you're relying on @safe in any significant way.
>
>
>> Is @safe supposed to solve all memory corruption or is it only if dip1000 and dip25 is on. So default @safe is not @safe?
>
> @safe is *intended* to prevent memory corruption.  But there are some loopholes in the current implementation, which are addressed by DIP1000 and DIP25.  AIUI, the intention is that -dip25 and -dip1000 will eventually be the default behaviour; we're just in the transition period right now because they could potentially break existing code (as in, they will cause compilation errors to otherwise working code).  So in the meantime, if your code doesn't break with -dip25 -dip1000, you should probably just compile with them by default in order to get the benefits of a better @safe implementation.
>
>
> T

Unfortunately, the code does break because of dependencies.

But anyway, I did some dip1000 experiments, and it does not seem to catch the auto ref issue:

@("Sanitary check")
unittest {
    auto ref get(T)(auto ref Optional!T value) {
        return value.front;
    }
    assert(get(some(1) == 1);
}

some is defined as:
public auto some(T)(auto ref T value) {
    return Optional!T(value);
}

And Optional.front is:

@property ref inout(T) front() return inout {
        assert(!empty, "Attempting to fetch the front of an empty optional.");
        return this._value;
    }

I've been running with ldc and ASAN enabled and:

==68300==ERROR: AddressSanitizer: stack-use-after-return on address 0x0001033a2d60 at pc 0x0001012d5006 bp 0x7ffeee967fb0 sp 0x7ffeee967fa8
READ of size 4 at 0x0001033a2d60 thread T0
    #0 0x1012d5005 in _D5tests8optional18__unittest_L512_C1FZv optional.d:520
    #1 0x10130cff2 in _D5tests8optional10__unittestZ optional.d
    #2 0x1013ba594 in main __entrypoint.d:8
    #3 0x7fff61a583d4 in start (libdyld.dylib:x86_64+0x163d4)

Address 0x0001033a2d60 is located in stack of thread T0 at offset 32 in frame
    #0 0x1012d51af in _D5tests8optional18__unittest_L512_C1FZ__T3getTiZQhMFNaNbNcNiNfSQCeQCh__T8OptionalTiZQmZi optional.d:514