Fix issue 10913 std.regex API regression

[DmitryOlshansky]

No idea why I can't package __traits(compiles, ...) in a trait-like template. I must be missing something ...

[quickfur]

Weird, why is it that encapsulating the test inside an enum doesn't work? Or is the problem because is(T==Captures!R) is not always true?

[DmitryOlshansky]

Just tried and put Captures!R instead of typeof(cap) - it works so the type (that is easy to trace) is correct.

[quickfur]

Hmm. The following reduced code does work:

struct Captures(R) {}
struct Regex(C) {}

enum isReplaceFunctor(alias fun, R) =
    __traits(compiles, (Captures!R c) { fun(c); });

void replace(alias output, R)(R input) {
    pragma(msg, __traits(compiles, (Captures!R c) { output(c); }));
    pragma(msg, isReplaceFunctor!(output, R));
}

void main() {
    Regex!char input;
    replace!((a) => "blah")(input);
}

It prints "true", "true". I'm going to try it with the actual regex code now to figure out why it's not working.

[quickfur]

Whoa, you will not believe this. Try replacing isReplaceFunctor with this version and it will work:

enum isReplaceFunctor(alias fun, R) =
    __traits(compiles, (Captures!R c) @system { fun(c); });

I think we should open a DMD bug for this.

[DmitryOlshansky]

I'm destroyed :)
Will do.

[DmitryOlshansky]

Done. Strangest fix in my book

[quickfur]

Yeah no kidding. The fix looks good.

But I think you should still file a bug against DMD for this. In theory, it should be able to infer the @safe-ty of the delegate automatically without requiring the @System.

[DmitryOlshansky]

Would be nice but demangle is not @safe...

[DmitryOlshansky]

In fact I just made any @System delegate callable via replace in @safe code as replace is @trusted... Shit..

[quickfur]

But isn't that already true in your original fix? Once you have an arbitrary lambda template parameter, in theory it can contain anything, including code that formats your disk and trashes your stack.

[quickfur]

On second thought, should replace be marked @trusted at all? It can't be @safe if you pass an unsafe delegate into it via the alias parameter. I thought the compiler can now infer @safety attributes, so all it takes is a @safe unittest to ensure that for reasonable lambdas replace is @safe. If user code wants to pass un@safe code to replace, it's not really your problem to try to make it somehow @safe.

On that note, this is one area where D is lacking: to have a way of saying "this function's safety depends on the safety of parameter X", similar to inout for const/unqual. Same goes for purity, nothrow, etc.. Sometimes I want to say "this function is pure, safe, nothrow, up to the purity/safety/nothrow-ness of delegate parameter X", but the current language has no way of expressing this.

[quickfur]

Actually, the latest DMD will already infer attributes for template functions, so you don't need to mark replace as @trusted at all. Here's a little test I did:

int globalVar;

void sysFunc() pure nothrow @system { int* p; (*p)++; }
void impureFunc() nothrow @safe { globalVar++; }
void throwingFunc() pure @safe { throw new Exception(""); }
void idealFunc() pure @safe nothrow { /* idealists love doing nothing :) */ }

void doStuff(alias fun)() {
    fun();
}

pragma(msg, "doStuff!sysFunc:\t", typeof(doStuff!sysFunc));
pragma(msg, "doStuff!impureFunc:\t", typeof(doStuff!impureFunc));
pragma(msg, "doStuff!throwingFunc:\t", typeof(doStuff!throwingFunc));
pragma(msg, "doStuff!idealFunc:\t", typeof(doStuff!idealFunc));

The compiler output is:

doStuff!sysFunc:    pure nothrow @system void()
doStuff!impureFunc: nothrow @safe void()
doStuff!throwingFunc:   pure @safe void()
doStuff!idealFunc:  pure nothrow @safe void()

So basically, if you omit attributes, the compiler will automatically infer the best attributes for replace().

[quickfur]

And I take back my comment about D being unable to express attributes based on parameters; the above inference already gives you the best possible attributes for each instantiation. But suppose you want a safeguard in your code so that future changes don't accidentally break purity, for example. It's possible to do this with today's language, though it may cause performance issues unless the compiler is sufficiently optimizing. You just wrap your code in a lambda literal that gets immediately called:

void doMoreStuff(alias fun)() {
    () pure {
        /* the compiler will ensure code in here is pure */
    }(); // call lambda immediately so it basically acts like a statement block
    fun(); // this call is outside the pure block so it still lets the compiler do its job with attribute inference
}

You can handle @safe and nothrow in the same way.

Once Kenji's pull dlang/dmd#2483 is merged, this approach can actually become practical without fearing performance hits.

[DmitryOlshansky]

Cool but then said isReplaceFunctor template has to be defined with whatever the current safety of replace instance is. ... Local templates can't take alias parameters....
It seems like we are stuck with inline traits(compiles) again?

[quickfur]

I'm just guessing, but I wonder if the original problem was caused because replace is @trusted, so the compiler tried to instantiate isReplaceFunctor as @safe? Because when I tested it outside of std.regex, the compiler was able to correctly compile (my copy of) isReplaceFunctor with no problems. Maybe try taking out @trusted from replace and see if the test case compiles?

[DmitryOlshansky]

If life ever was that easy... I've put @safe: on top of module and selectively marked stuff as @trusted... How to back off from @safe to "default mode" is anyone's bet :)
I will kill topmost safe but would then need to figure out what was meant to be safe in the end.

[DmitryOlshansky]

There wasn't much of public stuff anyway... updated.

[quickfur]

Seems like autotester is failing to compile the new code. :-(

[DmitryOlshansky]

Mmm is it my compiler is slightly out of date or something but it passes for me :)
Anyway tried to please it by putting @trusted where expected.

[DmitryOlshansky]

Damn it was green for a while before some wicked pull brought git master down :(