Bug in juce_Thread.h

Not sure if this is the right place to report this but in the latest JUCE build 7.0.6

juce_Thread.h line 87 is:

return withMember (*this, &RealtimeOptions::priority, juce::jlimit (newPriority, 0, 10));

It should be:

return withMember (*this, &RealtimeOptions::priority, juce::jlimit (0, 10, newPriority));

Strange that it wasn’t caught during development: any “newPriority” value greather than 0 would have triggered a jassert inside jlimit.

Maybe this method is never used in the current JUCE codebase, but definitely something that needs fixing

jlimit could really become std::clamp… I think C++17 has now arrived?

Doh! I’ll push a fix for this now - not sure how I let that slip through :man_facepalming:

Possibly I think I raised that a while ago there was a reason, but I’ll double check.

The good news is, for std::clamp the order of arguments was already correct :wink:
jlimit does it the other way round

Absolutely, it’s almost definitely why I made the mistake.

Sorry for the wait the fix for this made it into develop yesterday

I’m also trying to see if we can delicately deprecate jlimit in favour of std::clamp.

1 Like

Why don’t you rewrite that one line to use std::clamp instead of an outdated juce::jlimit?

IMHO all code should use as much standard stuff as possible and only use custom functions/classes if it provides clear benefits (additional functionality, performance, or security).

Jules even spearheaded the move to std::unique_ptr because it was just plain better than the juce implementation.

I can even see the benefit of something like juce::Array because it’s easier to use and offers extra features over std::vector.

But something like juce::jlimit? A stand-alone function with three parameters that offers no benefits and even has an utterly unfamiliar parameter-order is a no-brainer to replace. It should, at a minimum, be marked [[ deprecated ]]

I’m working on a separate commit to move to std::clamp, turns out it’s quite easy to make some mistakes while moving the parameter. Unfortunately not easily dealt with using regex. So I’m having a little play with writing a custom clang tool to hopefully help everyone move from jlimit to std::clamp, and I’m sure we could then use it to help users move to even more standard stuff.

So I looked at using deprecated and if our codebase is anything to go by it could be a big ask for users, hence I’m looking at tooling to help.

In principle though I completely agree with everything you’ve said.