| Message ID | 20221025235006.7215-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | target: Rename headers using .def extension to .h.inc | expand |
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > We use the .h.inc extension to include C headers. To be consistent > with the rest of the codebase, rename the C headers using the .def > extension. > > IDE/tools using our .editorconfig / .gitattributes will leverage > this consistency. Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On 27/10/22 08:46, Thomas Huth wrote: > On 26/10/2022 01.50, Philippe Mathieu-Daudé wrote: >> We use the .h.inc extension to include C headers. To be consistent >> with the rest of the codebase, rename the C headers using the .def >> extension. >> >> IDE/tools using our .editorconfig / .gitattributes will leverage >> this consistency. > > Ack for this series, but I've got a meta-question: Does anybody remember > why we are using .h.inc and not .inc.h for such headers? .h.inc has to > be manually configured in most editors for supporting syntax > highlighting here - with .inc.h most editors would get it right by > default instead... Daniel synthesized the reason here: https://lore.kernel.org/qemu-devel/20200817165207.GN4775@redhat.com/ >> IIRC, we need to use c.inc, because Meson has specific semantics >> around a file ending in ".c" that we don't want. First explanation from Paolo: https://lore.kernel.org/qemu-devel/36032642-9bea-8625-65a6-bd4afc7e459d@redhat.com/ See also for generic .*.inc admitted as convention: https://lore.kernel.org/qemu-devel/CAFEAcA-kOs3dKhh3SRchg6Ne8QL8kwyz+2ihDC6ND2v-seuRfw@mail.gmail.com/ Could be worth mentioning in docs/devel/build-system.rst... Regards, Phil.
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > We use the .h.inc extension to include C headers. To be consistent > with the rest of the codebase, rename the C headers using the .def > extension. > > IDE/tools using our .editorconfig / .gitattributes will leverage > this consistency. > > Philippe Mathieu-Daudé (3): > target/m68k: Rename qregs.def -> qregs.h.inc > target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc > target/tricore: Rename csfr.def -> csfr.h.inc > > target/m68k/{qregs.def => qregs.h.inc} | 0 > target/m68k/translate.c | 4 ++-- > target/s390x/tcg/{insn-data.def => insn-data.h.inc} | 2 +- > .../s390x/tcg/{insn-format.def => insn-format.h.inc} | 0 > target/s390x/tcg/translate.c | 10 +++++----- > target/tricore/{csfr.def => csfr.h.inc} | 0 > target/tricore/translate.c | 4 ++-- > 7 files changed, 10 insertions(+), 10 deletions(-) > rename target/m68k/{qregs.def => qregs.h.inc} (100%) > rename target/s390x/tcg/{insn-data.def => insn-data.h.inc} (99%) > rename target/s390x/tcg/{insn-format.def => insn-format.h.inc} (100%) > rename target/tricore/{csfr.def => csfr.h.inc} (100%) I wonder why we use any of .def, .h.inc, .inc.h, .c.inc, .inc.c. Why not .h and call it a day? No need to configure each and every editor to tread these as C code.
On Thu, 27 Oct 2022 at 15:40, Markus Armbruster <armbru@redhat.com> wrote: > I wonder why we use any of .def, .h.inc, .inc.h, .c.inc, .inc.c. Why > not .h and call it a day? No need to configure each and every editor to > tread these as C code. It says "this isn't actually a header in the usual sense". That's useful for automated scripted checks (eg we don't want scripts/clean-header-guards.pl to add the standard #include header guards to this sort of file) and for humans (if you see one of these files included as part of the normal #include block at the top of a .c file that's probably a mistake; if you see it being used then you know there's likely multiple-inclusion shenanigans going on.) thanks -- PMM
On 27/10/2022 16.12, Philippe Mathieu-Daudé wrote: > On 27/10/22 08:46, Thomas Huth wrote: >> On 26/10/2022 01.50, Philippe Mathieu-Daudé wrote: >>> We use the .h.inc extension to include C headers. To be consistent >>> with the rest of the codebase, rename the C headers using the .def >>> extension. >>> >>> IDE/tools using our .editorconfig / .gitattributes will leverage >>> this consistency. >> >> Ack for this series, but I've got a meta-question: Does anybody remember >> why we are using .h.inc and not .inc.h for such headers? .h.inc has to be >> manually configured in most editors for supporting syntax highlighting >> here - with .inc.h most editors would get it right by default instead... > > Daniel synthesized the reason here: > https://lore.kernel.org/qemu-devel/20200817165207.GN4775@redhat.com/ > > >> IIRC, we need to use c.inc, because Meson has specific semantics > >> around a file ending in ".c" that we don't want. > > First explanation from Paolo: > > https://lore.kernel.org/qemu-devel/36032642-9bea-8625-65a6-bd4afc7e459d@redhat.com/ > > > See also for generic .*.inc admitted as convention: > https://lore.kernel.org/qemu-devel/CAFEAcA-kOs3dKhh3SRchg6Ne8QL8kwyz+2ihDC6ND2v-seuRfw@mail.gmail.com/ > > Could be worth mentioning in docs/devel/build-system.rst... Thanks for the summary! ... now if someone (who feels confident about all of this) could add some sentences to build-system.rst, that would be really great... Thomas
Peter Maydell <peter.maydell@linaro.org> writes: > On Thu, 27 Oct 2022 at 15:40, Markus Armbruster <armbru@redhat.com> wrote: >> I wonder why we use any of .def, .h.inc, .inc.h, .c.inc, .inc.c. Why >> not .h and call it a day? No need to configure each and every editor to >> tread these as C code. > > It says "this isn't actually a header in the usual sense". That's > useful for automated scripted checks (eg we don't want > scripts/clean-header-guards.pl to add the standard #include header > guards to this sort of file) and for humans (if you see one of these > files included as part of the normal #include block at the top of > a .c file that's probably a mistake; if you see it being used then > you know there's likely multiple-inclusion shenanigans going on.) scripts/clean-header-guards.pl needs exclude patterns anyway. Comments would likely work better for humans than obscure naming conventions. Make them stylized, and they work for scripts, too.
On Thu, 27 Oct 2022 at 18:17, Markus Armbruster <armbru@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Thu, 27 Oct 2022 at 15:40, Markus Armbruster <armbru@redhat.com> wrote: > >> I wonder why we use any of .def, .h.inc, .inc.h, .c.inc, .inc.c. Why > >> not .h and call it a day? No need to configure each and every editor to > >> tread these as C code. > > > > It says "this isn't actually a header in the usual sense". That's > > useful for automated scripted checks (eg we don't want > > scripts/clean-header-guards.pl to add the standard #include header > > guards to this sort of file) and for humans (if you see one of these > > files included as part of the normal #include block at the top of > > a .c file that's probably a mistake; if you see it being used then > > you know there's likely multiple-inclusion shenanigans going on.) > > scripts/clean-header-guards.pl needs exclude patterns anyway. Yes, in theory instead of having a systematic convention for filenames we could instead give the files names that don't let you easily distinguish them from plain old header files and require every use like this to update clean-header-guards.pl, but that seems to me to be clearly worse than maintaining the filename convention that we already have. > Comments would likely work better for humans than obscure naming > conventions. > > Make them stylized, and they work for scripts, too. We already have a stylized convention, it's the filename... Comments inside the .inc file are also helpful, of course. -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Thu, 27 Oct 2022 at 18:17, Markus Armbruster <armbru@redhat.com> wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> > On Thu, 27 Oct 2022 at 15:40, Markus Armbruster <armbru@redhat.com> wrote: >> >> I wonder why we use any of .def, .h.inc, .inc.h, .c.inc, .inc.c. Why >> >> not .h and call it a day? No need to configure each and every editor to >> >> tread these as C code. >> > >> > It says "this isn't actually a header in the usual sense". That's >> > useful for automated scripted checks (eg we don't want >> > scripts/clean-header-guards.pl to add the standard #include header >> > guards to this sort of file) and for humans (if you see one of these >> > files included as part of the normal #include block at the top of >> > a .c file that's probably a mistake; if you see it being used then >> > you know there's likely multiple-inclusion shenanigans going on.) >> >> scripts/clean-header-guards.pl needs exclude patterns anyway. > > Yes, in theory instead of having a systematic convention for > filenames we could instead give the files names that don't > let you easily distinguish them from plain old header files and > require every use like this to update clean-header-guards.pl, > but that seems to me to be clearly worse than maintaining the > filename convention that we already have. Handle that just like for plain .h: when you re-run the script, you fix its complaints either by fixing the header or by adding it to the exclude pattern. >> Comments would likely work better for humans than obscure naming >> conventions. >> >> Make them stylized, and they work for scripts, too. > > We already have a stylized convention, it's the filename... True. But its opaque both for tools (editors mostly) and humans. Tools need to be configured, and humans need to be taught. Moreover, we don't have *a* stylized convention, we have *two*: funny filenames (several patterns, but that's fixable), and exclude patterns. > Comments inside the .inc file are also helpful, of course. Let's add a third! Stylized comments! I'll shut up now :)
We use the .h.inc extension to include C headers. To be consistent with the rest of the codebase, rename the C headers using the .def extension. IDE/tools using our .editorconfig / .gitattributes will leverage this consistency. Philippe Mathieu-Daudé (3): target/m68k: Rename qregs.def -> qregs.h.inc target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc target/tricore: Rename csfr.def -> csfr.h.inc target/m68k/{qregs.def => qregs.h.inc} | 0 target/m68k/translate.c | 4 ++-- target/s390x/tcg/{insn-data.def => insn-data.h.inc} | 2 +- .../s390x/tcg/{insn-format.def => insn-format.h.inc} | 0 target/s390x/tcg/translate.c | 10 +++++----- target/tricore/{csfr.def => csfr.h.inc} | 0 target/tricore/translate.c | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) rename target/m68k/{qregs.def => qregs.h.inc} (100%) rename target/s390x/tcg/{insn-data.def => insn-data.h.inc} (99%) rename target/s390x/tcg/{insn-format.def => insn-format.h.inc} (100%) rename target/tricore/{csfr.def => csfr.h.inc} (100%)