Make jassert use ignoreUnused internally

If you’re like me, you use a lot of jasserts while developing your project. However, when building release versions, all of those jasserts disappear.
if we’re doing something like:

auto f = someFunctionThatModifiesInternalState();
jassert( f < someValue );

The compiler is going to throw a warning that f is an unused variable during a release build.

it would be rad if jassert() internally used ignoreUnused(); to eat up those variables so they aren’t considered ‘unused’ anymore. I’m not sure how you could make that happen, since jassert expects an expression, and not individual variables, but it would be handy nonetheless. somethin’ to think about…

This is now available in the SR branch.

It’s so very useful and almost a one-liner.

(would had also voted but my votes are already spent)

3 Likes

I do have to mention the one theoretical down-side I’ve found for this: If the computation in the jassert is expensive and the compiler can’t know that it has no side effects (i.e it’s a call to a function from a different translation unit), then it will have runtime costs!

If this is an issue then perhaps it’s best to have two different assert calls. One that is debug only assert, and another which has a name that makes it clear that the code within does run (if it’s not a no-op).

3 Likes

This may be an issue, since JUCE itself uses jassert around std::find_if in some places. If you look at modules/juce_events/timers/juce_Timer.cpp in addTimer it checks whether the timer is already there.

I wanted to see whether we could simply do a cast-to-void as you sometimes see as a solution for this, e.g. on stackoverflow, but that doesn’t work, precisely because of things like that (lambdas are not allowed in unevaluated contexts).

1 Like

I think we probably can’t stick an ignoreUnused into jassert, for the reasons raised in this thread - it might break user code if Release mode builds were made to evaluate expressions that previously only evaluated in Debug mode.

I’m also not sure that implementing this in another way is feasible. As far as I know, there’s no way to unpack an expression and to only ignoreUnused the leaves of the expression. Perhaps this will change once reflection is added to C++, but it will be a very long time before JUCE is able to move to C++23 or higher.

What about something like this:

#define jassert(expression) JUCE_BLOCK_WITH_FORCED_SEMICOLON ( { constexpr bool __unused = false && (expression); (void)__unused; } )

This defines the __unused variable, which is compile-time evaluated to false. Short-circuiting will ensure that the expression isn’t actually executed. Then we need to ensure we don’t get warnings about __unused itself so we cast it to void.

3 Likes

That’s interesting, I’ll see if that works for the compilers we support. Thanks for the idea!

1 Like

I’ve tried it out on our own codebase. With some minor modifications, it seems to work well for us, so I have made a pull-request for it:

1 Like

I’ve been thinking about this a bit more, and although the change seems handy in general, I wonder whether it might occasionally end up suppressing some useful warnings.

As an example, occasionally a data member might be kept in a class solely to participate in jassert checks. If jassert always ‘uses’ this data member, we’ll never get a compiler warning telling us that the data member is unused. However, in this case the presence of the data member might have a real impact on performance, perhaps by reducing the number of objects which can fit in cache. The member would also add overhead to constructor calls, including copies and moves, so we might prefer to #if JUCE_DEBUG the data member away entirely when it is not used.

Although this might be a bit of an edge case, I’m not convinced that the potential downside of missed warnings and slower code is worth the benefit of slightly less typing.

2 Likes

Agreed, I do this quite a lot, adding extra data members simply for checking in asserts. In these cases I want the compiler to warn me when I haven’t debugged them out completely in release builds.

I guess that makes sense. Would it be an acceptable compromise to make that behaviour configurable, though? Given that there’s many people who consider this useful, that we could define something like JUCE_JASSERT_USES_VARIABLE or some such, which then takes care of “using” the variable in jassert even in release mode?

1 Like

I’ll have a look at adding something like jassert_ignore, as was suggested a bit further up. I think it’s probably safer to opt-in to this behaviour on a case-by-case basis, rather than having it be globally enabled/disabled.

4 Likes

Maybe I’m missing something here but wouldn’t it be simplest to just use [[ maybe_unused ]] when you declare you variable? E.g. from the original example:

[[ maybe_unused ]] auto f = someFunctionThatModifiesInternalState();
jassert( f < someValue );

Wouldn’t that work?

I think that would work, but it’s only available from C++17 onwards.

Is asking for this still on <17 then?

Are there platform specific variants on earlier compilers that could be wrapped in to a JUCE_MAYBE_UNUSED macro?

I didn’t find anything with a quick google. It looks like __attribute__((unused)) can be used on clang/gcc, but MSVC doesn’t have a C++14-compatible equivalent.

That’s the pattern we’ve been trying to avoid. With this your code can quickly become littered with many [[maybe_unused]] annotations, adding visual noise and reducing the noticeability of the attribute in other, more unusual cases.

+1

3 Likes

To add on @danradix reply, the problem with [[maybe_unused]] is that if later changes stop using the variable at all in assertions, you won’t get the unused warning because it still has the tag.

1 Like

I think this should apply to DBG() as well. The non-debug implementation of the DBG() macro is just blank from what I can see, so maybe it could change to something like

#define DBG(text) juce::ignoreUnused(text)

Although, I guess this has the same issues as mentioned about about ‘using’ a variable that’s not really used… So perhaps an additional DBG_ignore()?

1 Like

jassertquiet is now on develop.

I’m closing this topic to release spent votes.

4 Likes