Page MenuHomeFreeBSD

cdefs: Introduce __result_use_or_ignore_check
ClosedPublic

Authored by markj on Jan 12 2024, 10:20 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jul 5, 9:25 AM
Unknown Object (File)
Fri, Jul 5, 9:21 AM
Unknown Object (File)
Tue, Jun 25, 5:36 AM
Unknown Object (File)
Mon, Jun 24, 8:44 PM
Unknown Object (File)
Jun 3 2024, 3:38 AM
Unknown Object (File)
Apr 27 2024, 9:47 AM
Unknown Object (File)
Apr 27 2024, 9:47 AM
Unknown Object (File)
Apr 27 2024, 8:28 AM

Details

Summary

Try to paper over inconsistent semantics for __warn_unused_result__
between clang and gcc. See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 for a spirited
discussion of these semantics.

Introduce __result_use_or_ignore_check, which allows callers to
explicitly ignore the return value with a cast to void. Use that to
restore some checking for copyout() and friends, previously removed in
commit d07acc58d898 ("systm: Relax __result_use_check annotations").

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 55400
Build 52289: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Jan 13 2024, 12:52 AM

Based on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c56 we can use [[nodiscard]] with GCC 11 and higher which has the expected semantics. But for now this change seems fine

jhb added inline comments.
sys/sys/cdefs.h
338

If I read the GCC PR right, this can be written as something like:

#if __STDC_VERSION >= <insert C23 number>
#define __result_use_or_ignore_check    [[nodiscard]]
#elif defined(__clang__)
#define __result_use_or_ignore_check    __result_use_check
#elif __GNUC_PREREQ__(11, 0)
#define __result_use_or_ignore_check    [[nodiscard]]
#else
#define __result_use_or_ignore_check
#endif

However, I would 1) be inclined to spell this as __nodiscard, and 2) given we only support building the base system with GCC 12 or later now anyway, I would just make this the actual definition of __result_use_check. Or perhaps another way, I would still make the above a definition of __nodiscard and then keep __result_use_check as a compat alias for __nodiscard.

sys/sys/cdefs.h
338

I'm happy with renaming to __nodiscard. (Which itself is a confusing name given that you're explicitly allowed to discard return values using a void cast, but...)

The one minor drawback of your suggestion is that there is then no way to get gcc's stricter semantics. For some functions you really do want to force the caller to assign the return value. For example, I can't see how (void)copyin(...) ever makes sense. Yes, clang will allow it, but that's why we (ideally) compile with more than one compiler. I could be convinced that this is not worth bothering about, though.

This revision was automatically updated to reflect the committed changes.
sys/sys/cdefs.h
338

Sorry, I missed your earlier reply. My thinking is that nodiscard has been standardized and we should probably just stick with those semantics. I'm not really worried about people using (void) casts inappropriately to quiet the warning.