www.digitalmars.com         C & C++   DMDScript  

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

reply d-bugmail puremagic.com writes:
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 puremagic.com
        ReportedBy: zshazz gmail.com



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: -------
Dec 17 2013
parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=11758


Peter Alexander <peter.alexander.au gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |peter.alexander.au gmail.co
                   |                            |m



09:41:44 PST ---
I think 3 is the correct solution.

You are right in saying that it disallows legitimate use cases such as
"uniform(-2, 5u)", but currently these don't work, so we don't break any code.

You are also right in saying that it incurs a small performance hit but (a) it
is negligible compared to the generation of a random number, and (b) the user
can easily fix it by not using mixed signed/unsigned.

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