December 17, 2013 [Issue 11758] New: std.random.uniform fails when mixing signed/unsigned integrals | ||||
---|---|---|---|---|
| ||||
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 --- Comment #0 from Chris Cain <zshazz@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: ------- |
February 23, 2014 [Issue 11758] std.random.uniform fails when mixing signed/unsigned integrals | ||||
---|---|---|---|---|
| ||||
Posted in reply to Chris Cain | 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 --- Comment #1 from Peter Alexander <peter.alexander.au@gmail.com> 2014-02-23 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: ------- |
Copyright © 1999-2021 by the D Language Foundation