Why support negative ranges?

Some call sites may use a negative range. For example in addr relay, the timestamp of some address messages is randomly pushed into the past.

Also, it seems safer when the alternative is either unspecified behaviour, or a crash.

The behavior here still seems odd in those cases. I'd expect call sites that actually can result in negative range values to want "rand_uniform_duration(T)" to mean: "give me a uniform duration not exceeding T". If T is negative, that probably means just returning 0?

Yeah, the interval is determined at run-time and the passed in value can either mean the start or the end of the interval, with the other value being 0.

I am not sure if mapping any negative value to a constant 0 is less confusing. And I couldn't find another solution that is well defined at run time (no unspecified behaviour, no crash).

Yeah, I'm not objecting to this change. It just strikes me as bizarre semantics; the simplest interpretation of a range is specifying how wide the window of possible values is - and that simply cannot be negative.

An alternative is always returning range itself if it is <= 0. That at least satisfies the constraint "never return a value exceeding range", and probably works just as well for the intended call sites (who I suspect use to determine how long to wait)?

I think there is a minimal, theoretical advantage to disallowing negative values, as the inverting the most negative value is an integer sanitizer violation. Though, this shouldn't affect execution of a program that is not compiled with the integer sanitizer and shouldn't happen in practise either.

What is the "C++20" syntax, if I may ask?

With C++20 you can pass a duration to a template, and then do a static_assert on its value, so you'd write rand_uniform_duration<dur>() instead of rand_uniform_duration(dur) and get the compile-time check.

Oh, with dur you mean static_duration, not std::chrono::duration?

I can't get it to work with std::chrono::duration:

#include <chrono>

using namespace std::chrono_literals;

struct FRC {
    template <std::chrono::duration range>
    auto rand_uniform_delay(auto time) {
        return time + rand_uniform_duration<decltype(time)::duration, range>();
    }
    template <typename Duration, std::chrono::duration range>
    Duration rand_uniform_duration() noexcept {
        static_assert(range.count() > 0);
        return Duration{randrange(range.count())};
    }
};

int main() {
    FRC frc;
    frc.rand_uniform_delay<9h>(std::chrono::sys_seconds{1s});
    frc.rand_uniform_duration<std::chrono::seconds, 9h>();
}

https://godbolt.org/z/8Y868s98K

Hmm, I'm not really sure why it doesn't work (well, std::chrono::duration needs template parameters and stuff, but even after fixing that it doesn't work)

Here's something that does, via a wrapper class. Not sure that's any better than @theuni's c++17 compatible wrapper. https://godbolt.org/z/nnKx4fde5

#include <chrono>

int64_t randrange(int64_t x) { return x / 2; }

using namespace std::chrono_literals;

template<typename dur=std::chrono::seconds>
struct template_duration
{
    const int64_t count;
    using duration = dur;
    constexpr template_duration(dur f) : count{f.count()} { }
    constexpr dur to_dur() const { return dur{count}; }
};
template<typename dur>
constexpr template_duration<dur> tmpl_dur(const dur& d) { return {d}; }

struct FRC {
    template <typename Duration, auto range>
    Duration rand_uniform_duration() noexcept {
        static_assert(range.count > 0);
        return Duration{randrange(Duration{range.to_dur()}.count())};
    }

    template <auto range, typename X>
    X rand_uniform_delay(X time) noexcept {
        return time + rand_uniform_duration<typename X::duration, range>();
    }    
};

int main() {
    FRC frc;
    constexpr auto DELAY_HOURS{tmpl_dur(8h)};
    auto now = std::chrono::sys_seconds{1s};
    frc.rand_uniform_duration<std::chrono::seconds, DELAY_HOURS>();
    frc.rand_uniform_delay<DELAY_HOURS>(now);
}

Happy to use different std-lib syntax, but anything that requires a wrapper class that re-implements stuff seems overkill to solve a problem that doesn't exist.

review note: https://eel.is/c++draft/thread.condition.condvar#24