Re: Towards __deprecated in cdefs.h

From: Zhenlei Huang <zlei_at_FreeBSD.org>
Date: Fri, 26 Jul 2024 02:35:11 UTC

> On Jul 26, 2024, at 3:41 AM, Warner Losh <imp@bsdimp.com> wrote:
> 
> There's often times we want to mark interfaces as allowed, but please stop using it.

Also the userland libraries have some interfaces commented or documented deprecated but persist for quite long time.
It would be nice to have compiler warnings for the usage so we could evolve fast.

> 
> C23 has a new, fancy [[deprecated]] and [[deprecated("msg")]] forms. It would be nice to start using it.
> 
> The question is how.
> 
> Linux adopted, then effectively abandoned, __deprecated as a decorator. At first, it would produce warnings, but water was changed to be just ornamental because too many warnings were thrown during a kernel build.

Yes. Too many warnings to be useful.

I doubt if we can emit the warning for changed or new code only. I think it would be nice to have CI to do that. A typical usage should be pre-commit checks. Turn on flag of deprecated warnings only for the changed files and filter out the changed lines.

For existing code, if the migration is desired, developer could turn on the flag manually to verify his / her work.

> 
> So position [1] is to do what Linux did. Make iit a #define that does nothing.
> 
> Position [2] is do what Linux did originally: make it a warning to use deprecated interfaces (but likely a -Wno-error warning).
> 
> However, it would be nice sometimes to have a message that goes along with the use. Sadly, there's no way to have a macro in C or C++ that either takes an argument or doesn't. You either get pure replacement, or you get parameterized replacement, but never both. So, we'd need
> __deprecated_str or __deprecated_msg that took an optional message to give. This form would always warn, but could be paired with either [1] or [2] as [3] and [4].
> 
> Since we're talking about a macro that's slightly different than Linux, should it have a different name, in which case we'd have __dep and __dep_msg(x) as [5] and [6].

I think it is useful to always have a message to explain why and to direct an alternative .

> 
> There's likely more crazy, but that's likely too crazy to contemplate.
> 
> Personally, I think that option [4] is best: have __deprecated and __deprecated_msg(x), both of which always warn.
> 
> We don't need a __deprecated_error, I don't think. We get the same effect by removing it entirely, or removing its declaration from the .h file while keeping ot global.

No. Technically not. It would always be bypassed if intended. Why not turn __deprecated_error into ERROR or drop that definition ?

If deprecations would globally be turned to errors, the proper approach should a condition, either a macro, or compiler flag.

> 
> So before I spend a ton of time on implementing the various options, I thought I'd poll on arch@ to see if there's agreement that [4] is likely best, and if not, which other option I should put my weight behind. I realized I needed a wider discussion when I did [2] in https://reviews.freebsd.org/D46137 <https://reviews.freebsd.org/D46137> and the ensuing conversation in IRC didn't have 'no-brainer yes' written all over it.
> 
> The down side of doing [4] is we'll have to also change OpenZFS... but we likely should do that anyway since OpenZFS opted to use a copy of the linuxkpi compiler.h file rather than #include it and make minor mods :(. Maybe I'll make a patch for that too, or maybe I'll fix up https://github.com/openzfs/zfs/pull/16388 <https://github.com/openzfs/zfs/pull/16388>
> 
> Warner

Best regards,
Zhenlei