Date: Thu, 25 Jul 2024 17:57:12 -0600 From: Warner Losh <imp@bsdimp.com> To: Brooks Davis <brooks@freebsd.org> Cc: "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: Towards __deprecated in cdefs.h Message-ID: <CANCZdfq8RazcEX-wcwtVLdykxJS1841EM8nH9sbvyNNBXxeYyQ@mail.gmail.com> In-Reply-To: <ZqK_yJ7ARUFVQC1o@spindle.one-eyed-alien.net> References: <CANCZdfpBqNdAUK4R5YvQng3TpAPCUE%2B0ft5Q_xfRc6VuZoQWMw@mail.gmail.com> <ZqK_yJ7ARUFVQC1o@spindle.one-eyed-alien.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000f72425061e1b27f6
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
On Thu, Jul 25, 2024 at 3:12=E2=80=AFPM Brooks Davis <brooks@freebsd.org> w=
rote:
> On Thu, Jul 25, 2024 at 01:41:06PM -0600, Warner Losh wrote:
> > There's often times we want to mark interfaces as allowed, but please
> stop
> > using it.
> >
> > 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.
> >
> > 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 g=
et
> > 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].
> >
> > 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 effe=
ct
> > by removing it entirely, or removing its declaration from the .h file
> while
> > keeping ot global.
> >
> > 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 likel=
y
> > 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 and the ensuing conversation in IRC
> > didn't have 'no-brainer yes' written all over it.
>
> [4] with a message variant works for me. It's close to the standard thin=
g
> and makes it easy to see what you should be doing.
>
Yea. It also follows a few other things done as well...
> > 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
>
> IMO this file should be pared down to only things OpenZFS uses in
> shared code (__deprecated is not). It looks typical of the ZoL ->
> FreeBSD port in that overly broad shims where copied and hacked until
> the thing compiled, but then no effort was made to slim them down. See
> ecbf02791f9 in OpenZFS for another example.
>
Yea... The other way is to share better:
https://reviews.freebsd.org/D46144
and https://reviews.freebsd.org/D46145 so we share the fixes... I haven't
thought
about going the other direction... I'll have to see what that looks like.
I had the same problems with the original OpenZFS boot loader integration:
It barely compiled, but was super fragile and tiny changes would break
it again and again while I was testing (eg rebase forward a few weeks
while testing). So I rewrote it to make it way simpler...
Warner
--000000000000f72425061e1b27f6
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Thu, Jul 25, 2024 at 3:12=E2=80=AF=
PM Brooks Davis <<a href=3D"mailto:brooks@freebsd.org">brooks@freebsd.or=
g</a>> wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin=
:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"=
>On Thu, Jul 25, 2024 at 01:41:06PM -0600, Warner Losh wrote:<br>
> There's often times we want to mark interfaces as allowed, but ple=
ase stop<br>
> using it.<br>
> <br>
> C23 has a new, fancy [[deprecated]] and [[deprecated("msg")]=
] forms. It<br>
> would be nice to start using it.<br>
> <br>
> The question is how.<br>
> <br>
> Linux adopted, then effectively abandoned, __deprecated as a decorator=
. At<br>
> first, it would produce warnings, but water was changed to be just<br>
> ornamental because too many warnings were thrown during a kernel build=
.<br>
> <br>
> So position [1] is to do what Linux did. Make iit a #define that does<=
br>
> nothing.<br>
> <br>
> Position [2] is do what Linux did originally: make it a warning to use=
<br>
> deprecated interfaces (but likely a -Wno-error warning).<br>
> <br>
> However, it would be nice sometimes to have a message that goes along =
with<br>
> the use. Sadly, there's no way to have a macro in C or C++ that ei=
ther<br>
> takes an argument or doesn't. You either get pure replacement, or =
you get<br>
> parameterized replacement, but never both. So, we'd need<br>
> __deprecated_str or __deprecated_msg that took an optional message to =
give.<br>
> This form would always warn, but could be paired with either [1] or [2=
] as<br>
> [3] and [4].<br>
> <br>
> Since we're talking about a macro that's slightly different th=
an Linux,<br>
> should it have a different name, in which case we'd have __dep and=
<br>
> __dep_msg(x) as [5] and [6].<br>
> <br>
> There's likely more crazy, but that's likely too crazy to cont=
emplate.<br>
> <br>
> Personally, I think that option [4] is best: have __deprecated and<br>
> __deprecated_msg(x), both of which always warn.<br>
> <br>
> We don't need a __deprecated_error, I don't think. We get the =
same effect<br>
> by removing it entirely, or removing its declaration from the .h file =
while<br>
> keeping ot global.<br>
> <br>
> So before I spend a ton of time on implementing the various options, I=
<br>
> thought I'd poll on arch@ to see if there's agreement that [4]=
is likely<br>
> best, and if not, which other option I should put my weight behind. I<=
br>
> realized I needed a wider discussion when I did [2] in<br>
> <a href=3D"https://reviews.freebsd.org/D46137" rel=3D"noreferrer" targ=
et=3D"_blank">https://reviews.freebsd.org/D46137</a>; and the ensuing conver=
sation in IRC<br>
> didn't have 'no-brainer yes' written all over it.<br>
<br>
[4] with a message variant works for me.=C2=A0 It's close to the standa=
rd thing<br>
and makes it easy to see what you should be doing.<br></blockquote><div><br=
></div><div>Yea. It also follows a few other things done as well...</div><d=
iv>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0p=
x 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> The down side of doing [4] is we'll have to also change OpenZFS...=
but we<br>
> likely should do that anyway since OpenZFS opted to use a copy of the<=
br>
> linuxkpi compiler.h file rather than #include it and make minor mods :=
(.<br>
> Maybe I'll make a patch for that too, or maybe I'll fix up<br>
> <a href=3D"https://github.com/openzfs/zfs/pull/16388" rel=3D"noreferre=
r" target=3D"_blank">https://github.com/openzfs/zfs/pull/16388</a><br>;
<br>
IMO this file should be pared down to only things OpenZFS uses in<br>
shared code (__deprecated is not).=C2=A0 It looks typical of the ZoL -><=
br>
FreeBSD port in that overly broad shims where copied and hacked until<br>
the thing compiled, but then no effort was made to slim them down.=C2=A0 Se=
e<br>
ecbf02791f9 in OpenZFS for another example.<br></blockquote><div><br></div>=
<div>=C2=A0Yea...=C2=A0 The other way is to share better:=C2=A0<a href=3D"h=
ttps://reviews.freebsd.org/D46144">https://reviews.freebsd.org/D46144</a></=
div><div>and=C2=A0<a href=3D"https://reviews.freebsd.org/D46145">https://re=
views.freebsd.org/D46145</a> so we share the fixes... I haven't thought=
</div><div>about going the other direction...=C2=A0 I'll have to see wh=
at that looks like.</div><div><br></div><div>I had the same problems with t=
he original OpenZFS boot loader integration:</div><div>It barely compiled, =
but was super fragile and tiny changes would break</div><div>it again and a=
gain while I was testing (eg rebase forward a few weeks</div><div>while tes=
ting). So I rewrote it to make it way simpler...</div><div><br></div><div>W=
arner</div></div></div>
--000000000000f72425061e1b27f6--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfq8RazcEX-wcwtVLdykxJS1841EM8nH9sbvyNNBXxeYyQ>