Date: Thu, 25 Jul 2024 18:18:43 -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: <CANCZdfoFt634r9N261dF-vHXQPV6_uxSX_ptqhNAPUyGAjub9Q@mail.gmail.com> In-Reply-To: <CANCZdfq8RazcEX-wcwtVLdykxJS1841EM8nH9sbvyNNBXxeYyQ@mail.gmail.com> References: <CANCZdfpBqNdAUK4R5YvQng3TpAPCUE%2B0ft5Q_xfRc6VuZoQWMw@mail.gmail.com> <ZqK_yJ7ARUFVQC1o@spindle.one-eyed-alien.net> <CANCZdfq8RazcEX-wcwtVLdykxJS1841EM8nH9sbvyNNBXxeYyQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000f06c21061e1b7489
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
On Thu, Jul 25, 2024 at 5:57=E2=80=AFPM Warner Losh <imp@bsdimp.com> wrote:
>
>
> On Thu, Jul 25, 2024 at 3:12=E2=80=AFPM Brooks Davis <brooks@freebsd.org>=
wrote:
>
>> 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. I=
t
>> > 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
>> 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].
>> >
>> > 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.
>> >
>> > 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 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 thi=
ng
>> 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...
>
vs https://reviews.freebsd.org/D46146 which just starts to scratch the
surface.
There's a lot of other stuff that can likely be trimmed.
Warner
--000000000000f06c21061e1b7489
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 5:57=E2=80=AF=
PM Warner Losh <<a href=3D"mailto:imp@bsdimp.com">imp@bsdimp.com</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"><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=AFPM Brook=
s Davis <<a href=3D"mailto:brooks@freebsd.org" target=3D"_blank">brooks@=
freebsd.org</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" target=3D"_blank">https://reviews.freebs=
d.org/D46144</a></div><div>and=C2=A0<a href=3D"https://reviews.freebsd.org/=
D46145" target=3D"_blank">https://reviews.freebsd.org/D46145</a>; so we shar=
e the fixes... I haven't thought</div><div>about going the other direct=
ion...=C2=A0 I'll have to see what that looks like.</div><div><br></div=
><div>I had the same problems with the original OpenZFS boot loader integra=
tion:</div><div>It barely compiled, but was super fragile and tiny changes =
would break</div><div>it again and again while I was testing (eg rebase for=
ward a few weeks</div><div>while testing). So I rewrote it to make it way s=
impler...</div></div></div></blockquote><div><br></div><div>vs=C2=A0<a href=
=3D"https://reviews.freebsd.org/D46146">https://reviews.freebsd.org/D46146<=
/a> which just starts to scratch the surface.</div><div><br></div><div>Ther=
e's a lot of other stuff that can likely be trimmed.</div><div><br></di=
v><div>Warner=C2=A0</div></div></div>
--000000000000f06c21061e1b7489--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfoFt634r9N261dF-vHXQPV6_uxSX_ptqhNAPUyGAjub9Q>