std.datetime.systime - fix a -dip1000 compilable issue; trivial

[carblue]

This fixes a few
Error: reference to local variable ... assigned to non-scope parameter this
The only remaining issue in this module is with
Error: @safe function std.datetime.systime....._unittest... cannot call @System function std.stdio.writefln

[dlang-bot]

Thanks for your pull request and interest in making D better, @carblue! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

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.

[carblue]

I screwed the referred file: Correct: refers to std/datetime/systime.d

[wilzbach]

@carblue as mentioned in your other PR, I think it really makes sense to start adding -dip1000 to the CIs.
If you don't want to patch the Makefile, simply add a dip1000.sh file and call it in .circle/run.sh - that should be good enough for preventing regressions.
I'm thinking about something simple like:

modules=(
"std/algorithm"
)
for module in "${modules[@]}" ; do
   DFLAGS="-dip1000" "${module}.test"
done

Probably it makes sense to have a full list of all modules and then comment the ones which don't work.

Note that this requires a small patch to posix.mak:

diff --git a/posix.mak b/posix.mak
index ece00ee05..347885b1c 100644
--- a/posix.mak
+++ b/posix.mak
@@ -120,7 +120,7 @@ else
 endif
 
 # Set DFLAGS
-DFLAGS=-conf= -I$(DRUNTIME_PATH)/import $(DMDEXTRAFLAGS) -w -de -dip25 $(MODEL_FLAG) $(PIC) -transition=complex
+DFLAGS+=-conf= -I$(DRUNTIME_PATH)/import $(DMDEXTRAFLAGS) -w -de -dip25 $(MODEL_FLAG) $(PIC) -transition=complex
 ifeq ($(BUILD),debug)
 	DFLAGS += -g -debug
 else

(probably it should be more sophisticated)

[carblue]

@wilzbach
I'll submit a PR addressing phobos.posix.mak (I'm locally using a patched posix.mak with informative comments, referring to PR's and bugzilla issues; see also issue #18444; I think, it addresses exactly what You intend; it just needs to be completed to cover all phobos modules).
I'm not aware of the complete picture of the build process and all the consequences of such a phobos.posix.mak in CI though: It may perhaps prevent merging a legitimate fix in one file because it touches yet non(-dip1000/-unit-)tested code of another phobos module, now failing due to -dip1000. There is so much interrelation among phobos modules. writefln is a good example: Inferred as @safe with -dip25 and as @System with -dip1000 in some scenarios. You saw, that this std.stdio.writefln failure with -dip1000 now shows up in many other modules. Currently I have no solution to fix writefln.
But however You use that, it'll be good to have the place for documenting progress and infos.

[jmdavis]

I take it that scope is supposed to be here, because it returns by ref? If that's the case, then it makes more sense to me to put the scope on the left-hand side with the ref.

[carblue]

I checked combinations of omitting scope and it turned out to be dispensable here (but doesn't hurt either; seems to be inferred), but adjTime require it. See my comment there.

[jmdavis]

Please don't add scope if it's not required, especially since the DIP 1000 implementation isn't completely sorted out.

[carblue]

@jmdavis done: Removed scope here

[jmdavis]

What is the purpose of scope on either adjTime? They don't return by ref or accept anything by ref. I don't see any reason for scope to be involved.

[carblue]

All related errors reported with -dip1000 switch:
std/datetime/systime.d(2635): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.add!"years".add
std/datetime/systime.d(2846): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.add!"years".add
std/datetime/systime.d(3188): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.add!"months".add
std/datetime/systime.d(3535): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.add!"months".add
std/datetime/systime.d(4004): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"months".roll
std/datetime/systime.d(4383): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"months".roll
std/datetime/systime.d(4758): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"days".roll
std/datetime/systime.d(5019): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"hours".roll
std/datetime/systime.d(5230): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"minutes".roll
std/datetime/systime.d(5419): Error: reference to local variable sysTime assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"seconds".roll
std/datetime/systime.d(5554): Error: reference to local variable st assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"msecs".roll
std/datetime/systime.d(5684): Error: reference to local variable st assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"usecs".roll
std/datetime/systime.d(5826): Error: reference to local variable st assigned to non-scope parameter this calling std.datetime.systime.SysTime.roll!"hnsecs".roll

https://github.com/dlang/DIPs/blob/master/DIPs/DIP1000.md#scope-function-returns
In that section, the examples that have comment "applies to 'this' reference", thus scope on the left-hand side syntax requires e.g. a colon added as well (but doesn't work, seemingly unimplemented). The scope keyword (lifetime control) in all locations I touched here, is intended for the implicit this parameter. Btw, the fact that You - a well-known expert in D - are asking, supports my thesis that DIP1000 needs a better representation, otherwise it likely won't receive much adoption.

[jmdavis]

So, I guess that without scope, the compiler thinks that a pointer to the this reference could be escaping or something? Yuck. If that's the case, we could end up having to slap scope on most member functions. Either way, I favor using it as little as possible until DIP 1000 is fully sorted out.

Btw, the fact that You - a well-known expert in D - are asking, supports my thesis that DIP1000 needs a better representation, otherwise it likely won't receive much adoption.

I doubt that most of us really understand DIP 1000 properly, though I expect that some do. It's complicated, and it's not fully implemented. I read through it previously but not recently, and glancing over it now, it looks more complicated than I recall it being. Either way, it's making my brain hurt looking at it now. The attribute soup is already arguably too complicated as it is.

[carblue]

I had to read DIP1000.md a couple of times to have it somewhat stick in my brain (with all these different possible syntaxes for scope/return/ref +combinations) while I was working on PR #6041. It turned out - as Walter said in his 'pointers gone wild' talk - there where only a few places where I had to add scope for the implicit this parameter (i.e. not inferred currently; admittedly std.uni is heavily templated), but the remainder, Yes, needs slapping scope currently. There is often only 1 link of a chain missing and superfluous scope s can be removed again, as happened here.
And so far - while scanning phobos - I found only a few issues that separate us from -dip1000 compilable phobos; the memory corruption checks are better than their reputation; I created https://issues.dlang.org/show_bug.cgi?id=18444 in order to track missing implementation issues.