std.uni: add some scope and return

[WalterBright]

No description provided.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

[wilzbach]

You could also simply pull @carblue's existing work: #6041

[WalterBright]

Curiously, that doesn't add the same scope and return annotations.

[wilzbach]

Curiously, that doesn't add the same scope and return annotations.

BTW @carblue recently ran into issues when adding scope annotations to std.socket, because they result in breaking changes:

#6204

[WalterBright]

The circleci failure appears bogus.

[carblue]

Well, about 3 weeks ago I had - IIRC - the same impure errors with circleci and #6041 and could reproduce them with either make -f posix.mak std/uni.test or make -j6 -f posix.mak publictests. I don't recall exactly the route I took to get rid of them (GcPolicy.destroy pure, ForwardStrings.fwdStr @trusted being essential parts of the fix). In the end, for a good D-style, I removed all (/most of?) inferred attributes/storage class where I ever touched signatures. The posix.mak I used locally is in #6195. Another difficulty was, to have the std.uni code compile with both, either -dip25 or -dip1000.

[MartinNowak]

Quite a lot of @trusted around here.

[MartinNowak]

How about marking the mixed in fwdStr method as @safe/trusted and scope instead, then you could rely on inference and also remove the @trusted.

[WalterBright]

It's a good idea, but I am only concerned here with adding scope and return annotations.

[MartinNowak]

I was mostly asking because it's hard to seen whether fwdStr really does not escape anything, and adding scope to a @trusted function is unfortunately unchecked by the compiler.

[wilzbach]

The circleci failure appears bogus.
Well, about 3 weeks ago I had - IIRC - the same impure errors with circleci and #6041 and could reproduce them with either make -f posix.mak std/uni.test or make -j6 -f posix.mak publictests.

Yes you can even use make -f posix.mak std/uni.publictests (or in this case this target is failing: make -f posix.mak std/datetime/interval.publictests).
A bit of explanation publictests does nothing more than extracting all public unittests and puts them in a separate file (i.e. generated/linux/release/64/publictests/std_datetime_interval.d) and runs them.

Why? It was initially intended to ensure that all examples are runnable on dlang.org, but it's also an extra safety net against privacy issues and in your case, this user code would now stop to work:

void main()
{
    import std.datetime.interval;
    import std.datetime.date : Date, DayOfWeek;

    auto interval = Interval!Date(Date(2010, 9, 2), Date(2010, 9, 27));
    auto func = everyDayOfWeek!Date(DayOfWeek.mon);
    auto range = interval.fwdRange(func);
}

(simplified from the extracted public unittests)

[DmitryOlshansky]

The change is fine and compiler should be able to fix some unsafety I was concerned about.

[MartinNowak]

Errors seems about right InversionList.~this -> CowArray.~this -> ReallocPolicy.destroy!int -> core.stdc.stdlib.free isn't pure. Unclear why this pops up with scope related changes though.

[WalterBright]

Unclear why this pops up with scope related changes though.

It's also unclear why it would pass the unittests on the autotester and fail them on CircleCI.

[wilzbach]

It's also unclear why it would pass the unittests on the autotester and fail them on CircleCI.

Because the autotester just runs unittest which compiles everything into one big file whereas CircleCi (1) runs the unittest module-wise (e.g. std/algorithm/sorting.test) and (2) also extracts the public unittests in separate files and runs these with -unittest -main with the freshly built library (make -f posix.mak publictests). I don't know why these problems don't pop up in the normal build, but that's most likely a bug.

---

In short, auto-tester never tests this:

~/dlang/dmd/generated/linux/release/64/dmd -c -o- main.d

void main()
{
    import std.datetime.interval;
    import std.datetime.date : Date, DayOfWeek;

    auto interval = Interval!Date(Date(2010, 9, 2), Date(2010, 9, 27));
    auto func = everyDayOfWeek!Date(DayOfWeek.mon);
    auto range = interval.fwdRange(func);
}

[WalterBright]

So, I deleted all the scope annotations and now it compiles. I've been looking over the code in std.uni, and it seems nobody is able to deal with it. I'm not surprised, as there is pretty much zero documentation on how it works, which is especially problematic as it uses some obscure algorithms.

[WalterBright]

In short, auto-tester never tests this:

There's no obvious use of std.uni there.

[WalterBright]

fwdStr just seems to be an awful piece of code. The worst is, it does unsafe things, which prevents the compiler from checking it as @MartinNowak pointed out.

[WalterBright]

I'm going to go with #6041 and just abandon this.