[Issue 11758] New: std.random.uniform fails when mixing signed/unsigned integrals

[d-bugmail at puremagic.com]

https://d.puremagic.com/issues/show_bug.cgi?id=11758

           Summary: std.random.uniform fails when mixing signed/unsigned
                    integrals
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody at puremagic.com
        ReportedBy: zshazz at gmail.com


--- Comment #0 from Chris Cain <zshazz at gmail.com> 2013-12-17 13:09:44 PST ---
It appears that, despite accepting mixed signed/unsigned numbers,
std.random.uniform fails to handle them properly.

---
void main() {

    writeln(uniform(-1, 1u));
    // std.random.uniform(): invalid bounding interval [-1, 1)

    foreach(_; 0..5) {
        writeln(uniform(-2, uint.max));
        // Always prints out 4294967294
    }
}
---

This is problematic to fix. Possible fixes and a counter argument for each:

1. Rejecting mixing signed/unsigned numbers.
Issue: It may break a lot of code (std.random.randomShuffle, for instance, uses
`uniform(0, someRange.length))`)

2. Accepting mixing signed/unsigned numbers, but upsizing the return type to
the next largest signed integral type (so, promote byte/ubyte mixing to short,
short/uint mixing to long, etc.) and rejecting cases where promotion is
impossible (really just when one of the arguments is a long or ulong). This
would allow things like `uniform(int.min, uint.max)` to be meaningful and
actually return correct values.
Issue: Would still break std.random.randomShuffle and other code. Might work on
some code in 32-bit but cause that code to fail on 64-bit. It could also make
it so that error messages differ between 32-bit and 64-bit (changing where
failing code fails at, either at the usage of uniform or somewhere in client
code).

3. Accepting mixing signed/unsigned numbers, but doing a run-time check that
the signed number is non-negative.
Issue: Disallows many legitimate use cases (for instance, `uniform(-2, 5u)`
could return an int or long between -2 and 5 easily enough) unlike solution 2
and incurs an additional performance penalty for the check that would
necessitate a recommendation to change existing code anyway, without a way to
mechanically verify like the failure to compile as is in the case of solution
1.

4. Leave code as-is ... after all, it's been this way for quite awhile and no
one has run into this problem.
Issue: It's a silent ticking time bomb...

Any suggestions for a path to fix?

I think doing 1 but deprecating mixing signed/unsigned numbers for a few
releases prior might be the best. It goes without saying that the new
std.random could just do solution 1 and when code is migrated to the new
std.random it can be fixed at that time.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------