Re: [webkit-dev] maybe_unused vs UNUSED_PARAM
I support using standardized and widely enough available language features directly instead of through macros whenever we can. It’s similar to when we drop our own classes and use the ones from the C++ standard, which I think has been good for the project. It’s also fine with me to use the style checker script to catch mistakes and help educate contributors about this once we’ve decided. Not sure others agree with this, but I’d even support using global replace to move from old style to new style. One nice thing about language features is we don’t have to be so careful about not using them in headers. We don’t want to pull in lots of our own macros into headers that are used outside the project. It’s great for the long term health of the project to cut down the number of unique special things you need to know to understand and work with the code. I have two thoughts about possible reasons for caution as we do these transitions: Macros can be a flexible solution to cross-platform or cross-language issues. It would be a shame to move to a new language feature and discover only afterward that we as a project want to continue to support at least one compiler or platform that doesn’t have a good implementation yet, or support using some header from C, or that there was something else we can work around with macros. This would make me want to do some testing first on each of these before taking the plunge. We should be open to the possibility that some of our macros may still be better solutions to a problem than directly using the new language feature. For example, it is possible that ASSERT_UNUSED is slightly better for its purpose than [[maybe_unused]] since it has a more specific purpose. Marking the argument [[maybe_unused]] when we specifically know it’s only used for assertions isn’t perfect. Of course, ASSERT_UNUSED doesn’t quite do what we’d want either, but it’s still more specific than just saying “maybe”. The unused argument warning macros aren’t superb right now. It’s almost always better to leave out argument names instead of using them, because then there is no “maybe” about it. Unfortunately, the unused argument warning is mostly not turned on outside JavaScriptCore and WebCore. Also, it’s easy to have stale “unused” markings on things that are actually always used, so we leave behind macros that are both no longer needed and subtly misleading. I’m pretty sure [[maybe_unused]] is nearly identical in its properties to UNUSED_PARAM, so none of this really seems like an argument against that. And I’m not sure ASSERT_UNUSED is actually good enough to keep, despite these considerations. I agree with moving in the direction of using these. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] maybe_unused vs UNUSED_PARAM
Right, as long as it is part of the language and consistent across compilers / platforms, I don’t think we need to use macros. > On Jan 24, 2024, at 11:59 PM, Anne van Kesteren wrote: > > I had a [[fallthrough]] patch, but internal C code got in the way: > > - https://en.cppreference.com/w/c/language/attributes/fallthrough > - https://bugs.webkit.org/show_bug.cgi?id=265789 > > Using them directly where we can seems nice for (new) readers of the code at > least. Not sure what a macro for [[fallthrough]] would buy us for instance. > >> On Jan 25, 2024, at 12:28 AM, Ryosuke Niwa via webkit-dev >> wrote: >> >> If we’re adopting [[maybe_unused]], do we just write that directly in each >> function declaration / definition? Or do we define some a macro to do that >> anyway? >> >> What bout other kinds of attributes like [[noreturn]], [[fallthrough]], and >> [[likely]]? Are we gonna start writing them directly in code, or are we >> gonna continue to use macros? >> >> - R. NIwa >> >>> On Jan 24, 2024, at 9:49 AM, Chris Dumez via webkit-dev >>> wrote: >>> >>> Hi, >>> >>> Thanks for starting this discussion. >>> >>> I personally think it would be nice for us to switch to [[maybe_unused]] >>> since it is now part of the language and it seems to fit our needs. >>> However, I do think we should be consistent and stop using UNUSED_PARAM() / >>> ASSERT_UNUSED() in new code entirely then. >>> >>> So if we decide to switch, I think should add style checks to prevent using >>> UNUSED_PARAM() / ASSERT_UNUSED() and recommend using [[maybe_unused]] >>> instead. Eventually, we should try to phase out existing usage of these >>> macros so that we can remove them entirely. >>> >>> Cheers, >>> Chris. >>> On Jan 24, 2024, at 9:34 AM, Alex Christensen via webkit-dev wrote: For many years we have used the UNUSED_PARAM macros, and we have almost 3000 of them. C++17 introduced [[maybe_unused]] for this purpose, and a few uses of it are starting to pop up in WebKit. Should we switch, should we transition, should we allow both, or should we just stick with UNUSED_PARAM? ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev >>> >>> ___ >>> webkit-dev mailing list >>> webkit-dev@lists.webkit.org >>> https://lists.webkit.org/mailman/listinfo/webkit-dev >> >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> https://lists.webkit.org/mailman/listinfo/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] maybe_unused vs UNUSED_PARAM
I had a [[fallthrough]] patch, but internal C code got in the way: - https://en.cppreference.com/w/c/language/attributes/fallthrough - https://bugs.webkit.org/show_bug.cgi?id=265789 Using them directly where we can seems nice for (new) readers of the code at least. Not sure what a macro for [[fallthrough]] would buy us for instance. > On Jan 25, 2024, at 12:28 AM, Ryosuke Niwa via webkit-dev > wrote: > > If we’re adopting [[maybe_unused]], do we just write that directly in each > function declaration / definition? Or do we define some a macro to do that > anyway? > > What bout other kinds of attributes like [[noreturn]], [[fallthrough]], and > [[likely]]? Are we gonna start writing them directly in code, or are we gonna > continue to use macros? > > - R. NIwa > >> On Jan 24, 2024, at 9:49 AM, Chris Dumez via webkit-dev >> wrote: >> >> Hi, >> >> Thanks for starting this discussion. >> >> I personally think it would be nice for us to switch to [[maybe_unused]] >> since it is now part of the language and it seems to fit our needs. However, >> I do think we should be consistent and stop using UNUSED_PARAM() / >> ASSERT_UNUSED() in new code entirely then. >> >> So if we decide to switch, I think should add style checks to prevent using >> UNUSED_PARAM() / ASSERT_UNUSED() and recommend using [[maybe_unused]] >> instead. Eventually, we should try to phase out existing usage of these >> macros so that we can remove them entirely. >> >> Cheers, >> Chris. >> >>> On Jan 24, 2024, at 9:34 AM, Alex Christensen via webkit-dev >>> wrote: >>> >>> For many years we have used the UNUSED_PARAM macros, and we have almost >>> 3000 of them. C++17 introduced [[maybe_unused]] for this purpose, and a >>> few uses of it are starting to pop up in WebKit. Should we switch, should >>> we transition, should we allow both, or should we just stick with >>> UNUSED_PARAM? >>> ___ >>> webkit-dev mailing list >>> webkit-dev@lists.webkit.org >>> https://lists.webkit.org/mailman/listinfo/webkit-dev >> >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> https://lists.webkit.org/mailman/listinfo/webkit-dev > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] maybe_unused vs UNUSED_PARAM
If we’re adopting [[maybe_unused]], do we just write that directly in each function declaration / definition? Or do we define some a macro to do that anyway? What bout other kinds of attributes like [[noreturn]], [[fallthrough]], and [[likely]]? Are we gonna start writing them directly in code, or are we gonna continue to use macros? - R. NIwa > On Jan 24, 2024, at 9:49 AM, Chris Dumez via webkit-dev > wrote: > > Hi, > > Thanks for starting this discussion. > > I personally think it would be nice for us to switch to [[maybe_unused]] > since it is now part of the language and it seems to fit our needs. However, > I do think we should be consistent and stop using UNUSED_PARAM() / > ASSERT_UNUSED() in new code entirely then. > > So if we decide to switch, I think should add style checks to prevent using > UNUSED_PARAM() / ASSERT_UNUSED() and recommend using [[maybe_unused]] > instead. Eventually, we should try to phase out existing usage of these > macros so that we can remove them entirely. > > Cheers, > Chris. > >> On Jan 24, 2024, at 9:34 AM, Alex Christensen via webkit-dev >> wrote: >> >> For many years we have used the UNUSED_PARAM macros, and we have almost 3000 >> of them. C++17 introduced [[maybe_unused]] for this purpose, and a few uses >> of it are starting to pop up in WebKit. Should we switch, should we >> transition, should we allow both, or should we just stick with UNUSED_PARAM? >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> https://lists.webkit.org/mailman/listinfo/webkit-dev > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] maybe_unused vs UNUSED_PARAM
Hi, Thanks for starting this discussion. I personally think it would be nice for us to switch to [[maybe_unused]] since it is now part of the language and it seems to fit our needs. However, I do think we should be consistent and stop using UNUSED_PARAM() / ASSERT_UNUSED() in new code entirely then. So if we decide to switch, I think should add style checks to prevent using UNUSED_PARAM() / ASSERT_UNUSED() and recommend using [[maybe_unused]] instead. Eventually, we should try to phase out existing usage of these macros so that we can remove them entirely. Cheers, Chris. > On Jan 24, 2024, at 9:34 AM, Alex Christensen via webkit-dev > wrote: > > For many years we have used the UNUSED_PARAM macros, and we have almost 3000 > of them. C++17 introduced [[maybe_unused]] for this purpose, and a few uses > of it are starting to pop up in WebKit. Should we switch, should we > transition, should we allow both, or should we just stick with UNUSED_PARAM? > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] maybe_unused vs UNUSED_PARAM
For many years we have used the UNUSED_PARAM macros, and we have almost 3000 of them. C++17 introduced [[maybe_unused]] for this purpose, and a few uses of it are starting to pop up in WebKit. Should we switch, should we transition, should we allow both, or should we just stick with UNUSED_PARAM? ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
