Add attributes to core.thread

[PetarKirov]

While working on adding attributes to std.random (and in particular unpredictableSeed) in PR dlang/phobos#4136, I noticed that much of the functions in core.thread don't have appropriate attributes (in particular Thread.getThis), so I added them where possible.

As a result, after going through the whole module, here's what's left before the core.thread can be
fully @nogc and nothrow:

• thread_entryPoint
  -> calls Thread.run, which is not @nogc and nothrow

• thread_suspendHandler
  -> calls callWithStackShell which takes a non-@nogc delegate

• Thread.start
  -> calls onThreadError which is not @nogc

• Thread.id/name/isDeamon
  -> synchronized is not nothrow

• Thread.priority
  -> throws new ThreadException

• Thread.run
  -> User's callback has no attributes

• thread_init
  -> calls Thread.initLocks

• thread_term
  -> calls Thread.termLocks

• Thread.initLocks/termLocks
  -> calls Mutex.this/~this which are not @nogc and nothrow

• thread_attachThis/thread_attachByAddr/thread_attachByAddrB
  -> Create a new Thread instance with the GC

• thread_detachByAddr
  -> thread_findByAddr is not @nogc and nothrow

• thread_findByAddr
  -> Thread.opApply is not @nogc and nothrow

• Thread.getAll/opApply/getAllImpl
  -> calls a non-@nogc non-nothrow delegate
  -> Thread.getAll returns a GC-ed array

• thread_joinAll
  -> calls Thread.join which is not @nogc and nothrow
  -> calls Thread.isDeamon not nothrow

• Thread.join
  -> throws new ThreadException

• thread_suspendAll
  -> calls Thread.suspend which not @nogc
  -> calls onThreadError which is not @nogc

• Thread.suspend
  -> calls onThreadError which is not @nogc

• thread_resumeAll
  -> calls Thread.resume which is not @nogc

• Thread.resume
  -> calls onThreadError which is not @nogc

• thread_scanAll/thread_scanAllType/scanAllTypeImpl
  -> take a non-@nogc delegate
  -> call rt_tlsgc_scan which also takes a non-@nogc delegate

• thread_enterCriticalRegion/thread_exitCriticalRegion/thread_inCriticalRegion
  -> uses synchronized which is not nothrow

• thread_processGCMarks
  -> calls a non-@nogc delegate

• ThreadGroup.*
  -> uses associative array under synchronized

• Fiber.this
  -> calls allocStack which is not @nogc

• allocStack
  -> allocates a new Thread.Context from with the GC

• Fiber.call
  -> calls fiber_entryPoint

• fiber_entryPoint
  -> calls Fiber.run which is not @nogc

• Fiber.run
  -> calls arbitrary user callback

---

In summary, most of these fall into several categories:

• calls onThreadError which is not @nogc ->

  • We should fix onThreadError, to not use the GC, probably stepping on Make AssertError a singleton #1710

• throws new ThreadException ->

  • Should be changed to use the future version of onThreadError

• calls Mutex.this/~this which are not @nogc and nothrow ->

  • We should add attributes to core.sync.* - done in Makes core.sync.mutex.Mutex usable in modern code #1728

• synchronized is not nothrow ->

  • We should fix _d_monitorenter and _d_monitorexit

• returns GC allocated memory to the user (e.g. thread_attachThis, Thread.getAll) ->

  • Add overload / variation which takes an allocator as a parameter

• Internally allocates GC memory (e.g. Fiber's allocStack) ->

  • Should manually manage memory

• Internal function calls an internal non-@nogc callback ->

  • Should make both the callback and the internal function @nogc

• Calls a user provided callback.That's probably the trickiest. ->

  • One solution is to add DerivedThread(std.traits.FunctionAttribute Attr) and DerivedFiber(std.traits.FunctionAttribute Attr) templates, so that Thread.runandFiber.call` would propagate the attributes.

• ThreadGroup is not nothrow @nogc ->

  • it should be rewritten to use non-GC backed associative array and to use @nogc nothrow mutex (see Makes core.sync.mutex.Mutex usable in modern code #1728)

[PetarKirov]

@jmdavis @schveiguy @rainers @MartinNowak please take a look. The diff is a bit large but otherwise all of the changes should be easy to review.

@joakim-noah is there an easy way (preferably without messing with LDC) to test my change for version (Android)?

[joakim-noah]

No, I'll try it and let you know.

[PetarKirov]

Thanks @joakim-noah, much appreciated!

[wilzbach]

While working on adding attributes to std.random (and in particular unpredictableSeed)

FYI there would have been a different way: letting unpredictableSeed query the OS for random data - see e.g. libmir/mir-random#13

Anyways this is great work!

[PetarKirov]

FYI there would have been a different way: letting unpredictableSeed query the OS for random data - see e.g. libmir/mir-random#13

Yeah, that's probably a better approach, but I wanted to finish my PR, which fortunately led me to this one and #1728.

Anyways this is great work!

Thanks, though IMO #1728 is much interesting and important, because it makes shared much more usable in @safe code. Personally I think that shared is well designed from a language point of view, it's just the implementation that's lacking - primarily core.sync.* doesn't respect shared and synchronized doesn't peel off one level of shared-ness from the type (oh and synchronized (obj1, obj2) is yet not implemented). Currently the situation feels like making a legacy C++ code base const-correct - it's a lot of work, but most of the changes are close to trivial individually, and it pays off in the long run.

[joakim-noah]

Regarding Android compatibility, I tried modifying this PR to apply against druntime from ldc master, which is from the 2.071 branch. However, it crapped out when compiling with a bunch of attribute errors in core.thread, guessing it's related to being behind druntime master. I doubt adding a bunch of attributes to rt.sections_android is going to change much, especially if you're trying to match the signatures everywhere else, can always fix it later if it does.

[PetarKirov]

Ping @klickverbot @ibuclaw @MartinNowak I'd like to move forward with this PR. Is there anything else I need to do?

[andralex]

Good stuff!

[PetarKirov]

Thanks!

[MartinNowak]

You gotta hate attributes.
@property uint index() nothrow pure @nogc { return _index; }

Hope we get to that 2017H2, Mitigating the attribute proliferation - attribute inference for functions - D Programming Language Discussion Forum.

[MartinNowak]

This broke at least vibe.d and dependents, we need to be really careful with API's of non-final classes.
It's such a maintenance liability that I'd currently recommend to avoid non-final classes for any future additions to druntime/phobos. For the time being though we have to deal with what's there.

Unfortunately the current project tester didn't cover this b/c ZombineDev wasn't yet whitelisted (ci2 won't need whitelists ¹).

Related to class compatibility and deprecations.
DIP: "future symbol" compiler concept

[andralex]

@MartinNowak how do Java and C# (of which standard and other libraries rely heavily on non-final classes) deal with such breakages?

[MartinNowak]

Guess they don't have such an attribute craze like us.

[MartinNowak]

@andralex

[MartinNowak]

They do have related issues, hence the addition of default interface function implementations to Java.
Default Methods (The Java™ Tutorials > Learning the Java Language > Interfaces and Inheritance).

[PetarKirov]

@MartinNowak Do you know of any other issues than the one reported in https://issues.dlang.org/show_bug.cgi?id=17130. Or is it just PR #1728 the culprit?

[PetarKirov]

@MartinNowak how do I get myself whitelisted and how can I join dlang.slack.com?