Date: Fri, 26 Jul 2024 10:35:11 +0800 From: Zhenlei Huang <zlei@FreeBSD.org> To: Warner Losh <imp@bsdimp.com> Cc: "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: Towards __deprecated in cdefs.h Message-ID: <4480E3AA-106D-40C3-84FF-7C6A11EC2649@FreeBSD.org> In-Reply-To: <CANCZdfpBqNdAUK4R5YvQng3TpAPCUE%2B0ft5Q_xfRc6VuZoQWMw@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
> 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
[-- Attachment #2 --]
<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jul 26, 2024, at 3:41 AM, Warner Losh <<a href="mailto:imp@bsdimp.com" class="">imp@bsdimp.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">There's often times we want to mark interfaces as allowed, but please stop using it.</div></div></blockquote><div><br class=""></div><div>Also the userland libraries have some interfaces commented or documented deprecated but persist for quite long time.</div><div>It would be nice to have compiler warnings for the usage so we could evolve fast.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">C23 has a new, fancy [[deprecated]] and [[deprecated("msg")]] forms. It would be nice to start using it.</div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">The question is how.</div><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote><div><br class=""></div><div>Yes. Too many warnings to be useful.</div><div><br class=""></div><div>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 <span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class="">warnings only for the changed files and filter out the changed lines.</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class=""><br class=""></span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class="">For existing code, if the migration is desired, developer could turn on the flag manually to verify his / her work.</span></div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">So position [1] is to do what Linux did. Make iit a #define that does nothing.</div><div class=""><br class=""></div><div class="">Position [2] is do what Linux did originally: make it a warning to use deprecated interfaces (but likely a -Wno-error warning).</div><div class=""><br class=""></div><div class="">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</div><div class="">__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].</div><div class=""><br class=""></div><div class="">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].</div></div></div></blockquote><div><br class=""></div><div>I think it is useful to always have a message to explain why and to direct an alternative .</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">There's likely more crazy, but that's likely too crazy to contemplate.</div><div class=""><br class=""></div><div class="">Personally, I think that option [4] is best: have __deprecated and __deprecated_msg(x), both of which always warn.</div><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote><div><br class=""></div><div>No. Technically not. It would always be bypassed if intended. Why not turn __deprecated_error into ERROR or drop that definition ?</div><div><br class=""></div><div>If deprecations would globally be turned to errors, the proper approach should a condition, either a macro, or compiler flag.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">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 <a href="https://reviews.freebsd.org/D46137" class="">https://reviews.freebsd.org/D46137</a>; and the ensuing conversation in IRC didn't have 'no-brainer yes' written all over it.</div><div class=""><br class=""></div><div class="">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 <a href="https://github.com/openzfs/zfs/pull/16388" class="">https://github.com/openzfs/zfs/pull/16388</a></div><div class=""><br class=""></div><div class="">Warner</div></div>
</div></blockquote></div><br class=""><div class="">
<div>Best regards,</div><div>Zhenlei</div>
</div>
<br class=""></body></html>
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4480E3AA-106D-40C3-84FF-7C6A11EC2649>