[phobos] improved module cycle detection algorithm finds existing cycle in phobos, what to do?

[Andrei Alexandrescu]

On 11/5/10 11:21 AM, Steve Schveighoffer wrote:
> I don't know enough about std.random and std.encoding to know what to do with
> them.  Does anyone own these?

Janice wrote std.encoding and I wrote (most of) std.random. Anyway, both 
are now "Phobos owned" :o).

> I think the cycle as I print it is pretty clear.  What do you find unclear about
> it?

Well I didn't look into this closely but essentially there was this list:

Cyclic dependency in module std.encoding
imported from std.string
imported from std.dateparse
imported from std.date
imported from std.file
imported from std.stdio
imported from std.functional
imported from std.range
imported from std.exception
imported from std.conv
imported from std.array
imported from std.algorithm
imported from std.random *
imported from std.range
imported from std.exception
imported from std.conv
imported from std.array
imported from std.algorithm
imported from std.string
imported from std.encoding *
object.Exception: Aborting due to cycle between (*) elements with module 
ctors/dtors

It didn't immediately give away where the problem is and what steps need 
to be taken to fix it. Probably that's why you were compelled to add a 
natural language translation:

> So what this is saying, is std.encoding indirectly imports std.random, which
> indirectly imports std.encoding, and both of these modules contain shared module
> ctors.  I manually verified the cycle path, and that shared ctors exist, so it
> is a real cycle.

I also find the passive tense backwards. So I guess I'd format the 
message as follows:

=============
Cyclic dependency: std.encoding imports std.random and vice versa, and 
both define module constructors. Details ("<-" means "imports"):

std.encoding <- std.string <- std.algorithm <- ... <- std.random
std.random <- std.algorithm <- ... <- std.encoding
=============

Long chains can be improved by e.g. reformatting to one import per line:

std.encoding <- std.string
              <- std.algorithm
              <- ...
              <- std.random
std.random <- std.algorithm
            <- ...
            <- std.encoding

Just a thought.

One way to break a dependency chain is by defining a module that both 
std.encoding and std.random include.

Anyway, currently the static ctor in std.random is:

shared static this()
{
     ulong s;

     version(Win32)
     {
         QueryPerformanceCounter(&s);
     }
     version(Posix)
     {
         // time.h
         // sys/time.h

         timeval tv;
         if (gettimeofday(&tv, null))
         {   // Some error happened - try time() instead
             s = core.sys.posix.sys.time.time(null);
         }
         else
         {
             s = cast(ulong)((cast(long)tv.tv_sec << 32) + tv.tv_usec);
         }
     }
     //rand_seed(cast(uint) s, cast(uint)(s >> 32));
}

As the last line is already commented out _and_ rand_seed is deprecated 
anyway, you may want to simply remove the constructor!

Andrei