All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] target: Rename headers using .def extension to .h.inc
@ 2022-10-25 23:50 Philippe Mathieu-Daudé
  2022-10-25 23:50 ` [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc Philippe Mathieu-Daudé
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-25 23:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Laurent Vivier, Cornelia Huck, qemu-trivial,
	Philippe Mathieu-Daudé

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%)

-- 
2.37.3



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc
  2022-10-25 23:50 [PATCH 0/3] target: Rename headers using .def extension to .h.inc Philippe Mathieu-Daudé
@ 2022-10-25 23:50 ` Philippe Mathieu-Daudé
  2022-10-26  9:54   ` Laurent Vivier
  2022-11-02 18:29   ` Laurent Vivier
  2022-10-25 23:50 ` [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-25 23:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Laurent Vivier, Cornelia Huck, qemu-trivial,
	Philippe Mathieu-Daudé

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/m68k/{qregs.def => qregs.h.inc} | 0
 target/m68k/translate.c                | 4 ++--
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename target/m68k/{qregs.def => qregs.h.inc} (100%)

diff --git a/target/m68k/qregs.def b/target/m68k/qregs.h.inc
similarity index 100%
rename from target/m68k/qregs.def
rename to target/m68k/qregs.h.inc
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 9df17aa4b2..f018fa9eb0 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -39,7 +39,7 @@
 
 #define DEFO32(name, offset) static TCGv QREG_##name;
 #define DEFO64(name, offset) static TCGv_i64 QREG_##name;
-#include "qregs.def"
+#include "qregs.h.inc"
 #undef DEFO32
 #undef DEFO64
 
@@ -75,7 +75,7 @@ void m68k_tcg_init(void)
 #define DEFO64(name, offset) \
     QREG_##name = tcg_global_mem_new_i64(cpu_env, \
         offsetof(CPUM68KState, offset), #name);
-#include "qregs.def"
+#include "qregs.h.inc"
 #undef DEFO32
 #undef DEFO64
 
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc
  2022-10-25 23:50 [PATCH 0/3] target: Rename headers using .def extension to .h.inc Philippe Mathieu-Daudé
  2022-10-25 23:50 ` [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc Philippe Mathieu-Daudé
@ 2022-10-25 23:50 ` Philippe Mathieu-Daudé
  2022-10-27  6:45   ` Thomas Huth
  2022-11-02 18:30   ` Laurent Vivier
  2022-10-25 23:50 ` [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-25 23:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Laurent Vivier, Cornelia Huck, qemu-trivial,
	Philippe Mathieu-Daudé

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 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 +++++-----
 3 files changed, 6 insertions(+), 6 deletions(-)
 rename target/s390x/tcg/{insn-data.def => insn-data.h.inc} (99%)
 rename target/s390x/tcg/{insn-format.def => insn-format.h.inc} (100%)

diff --git a/target/s390x/tcg/insn-data.def b/target/s390x/tcg/insn-data.h.inc
similarity index 99%
rename from target/s390x/tcg/insn-data.def
rename to target/s390x/tcg/insn-data.h.inc
index 6382ceabfc..7e952bdfc8 100644
--- a/target/s390x/tcg/insn-data.def
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -8,7 +8,7 @@
  *
  *  OPC  = (op << 8) | op2 where op is the major, op2 the minor opcode
  *  NAME = name of the opcode, used internally
- *  FMT  = format of the opcode (defined in insn-format.def)
+ *  FMT  = format of the opcode (defined in insn-format.h.inc)
  *  FAC  = facility the opcode is available in (defined in DisasFacility)
  *  I1   = func in1_xx fills o->in1
  *  I2   = func in2_xx fills o->in2
diff --git a/target/s390x/tcg/insn-format.def b/target/s390x/tcg/insn-format.h.inc
similarity index 100%
rename from target/s390x/tcg/insn-format.def
rename to target/s390x/tcg/insn-format.h.inc
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 1d2dddab1c..f378e1a633 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -1011,7 +1011,7 @@ static void free_compare(DisasCompare *c)
 #define F6(N, X1, X2, X3, X4, X5, X6) F0(N)
 
 typedef enum {
-#include "insn-format.def"
+#include "insn-format.h.inc"
 } DisasFormat;
 
 #undef F0
@@ -1076,7 +1076,7 @@ typedef struct DisasFormatInfo {
 #define F6(N, X1, X2, X3, X4, X5, X6)       { { X1, X2, X3, X4, X5, X6 } },
 
 static const DisasFormatInfo format_info[] = {
-#include "insn-format.def"
+#include "insn-format.h.inc"
 };
 
 #undef F0
@@ -6143,7 +6143,7 @@ static void in2_insn(DisasContext *s, DisasOps *o)
 #define E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D, FL) insn_ ## NM,
 
 enum DisasInsnEnum {
-#include "insn-data.def"
+#include "insn-data.h.inc"
 };
 
 #undef E
@@ -6223,7 +6223,7 @@ enum DisasInsnEnum {
 #define FAC_MIE3        S390_FEAT_MISC_INSTRUCTION_EXT3 /* miscellaneous-instruction-extensions facility 3 */
 
 static const DisasInsn insn_info[] = {
-#include "insn-data.def"
+#include "insn-data.h.inc"
 };
 
 #undef E
@@ -6233,7 +6233,7 @@ static const DisasInsn insn_info[] = {
 static const DisasInsn *lookup_opc(uint16_t opc)
 {
     switch (opc) {
-#include "insn-data.def"
+#include "insn-data.h.inc"
     default:
         return NULL;
     }
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc
  2022-10-25 23:50 [PATCH 0/3] target: Rename headers using .def extension to .h.inc Philippe Mathieu-Daudé
  2022-10-25 23:50 ` [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc Philippe Mathieu-Daudé
  2022-10-25 23:50 ` [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc Philippe Mathieu-Daudé
@ 2022-10-25 23:50 ` Philippe Mathieu-Daudé
  2022-10-26  8:21   ` Bastian Koppelmann
  2022-11-02 18:30   ` Laurent Vivier
  2022-10-26  9:45 ` [PATCH 0/3] target: Rename headers using .def extension to .h.inc Alex Bennée
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-25 23:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Laurent Vivier, Cornelia Huck, qemu-trivial,
	Philippe Mathieu-Daudé

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/tricore/{csfr.def => csfr.h.inc} | 0
 target/tricore/translate.c              | 4 ++--
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename target/tricore/{csfr.def => csfr.h.inc} (100%)

diff --git a/target/tricore/csfr.def b/target/tricore/csfr.h.inc
similarity index 100%
rename from target/tricore/csfr.def
rename to target/tricore/csfr.h.inc
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index a0558ead71..f02090945d 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -388,7 +388,7 @@ static inline void gen_mfcr(DisasContext *ctx, TCGv ret, int32_t offset)
         gen_helper_psw_read(ret, cpu_env);
     } else {
         switch (offset) {
-#include "csfr.def"
+#include "csfr.h.inc"
         }
     }
 }
@@ -418,7 +418,7 @@ static inline void gen_mtcr(DisasContext *ctx, TCGv r1,
             gen_helper_psw_write(cpu_env, r1);
         } else {
             switch (offset) {
-#include "csfr.def"
+#include "csfr.h.inc"
             }
         }
     } else {
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc
  2022-10-25 23:50 ` [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc Philippe Mathieu-Daudé
@ 2022-10-26  8:21   ` Bastian Koppelmann
  2022-11-02 18:30   ` Laurent Vivier
  1 sibling, 0 replies; 18+ messages in thread
From: Bastian Koppelmann @ 2022-10-26  8:21 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, David Hildenbrand, Thomas Huth, qemu-s390x,
	Richard Henderson, Laurent Vivier, Cornelia Huck, qemu-trivial

On Wed, Oct 26, 2022 at 01:50:06AM +0200, 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/tricore/{csfr.def => csfr.h.inc} | 0
>  target/tricore/translate.c              | 4 ++--
>  2 files changed, 2 insertions(+), 2 deletions(-)
>  rename target/tricore/{csfr.def => csfr.h.inc} (100%)

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
  2022-10-25 23:50 [PATCH 0/3] target: Rename headers using .def extension to .h.inc Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2022-10-25 23:50 ` [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc Philippe Mathieu-Daudé
@ 2022-10-26  9:45 ` Alex Bennée
       [not found] ` <8d197b71-937a-5693-3b7f-ea4bded8c360@redhat.com>
  2022-10-27 14:39 ` Markus Armbruster
  5 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2022-10-26  9:45 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Laurent Vivier, Cornelia Huck, qemu-trivial,
	qemu-devel


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>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc
  2022-10-25 23:50 ` [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc Philippe Mathieu-Daudé
@ 2022-10-26  9:54   ` Laurent Vivier
  2022-11-02 18:29   ` Laurent Vivier
  1 sibling, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2022-10-26  9:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Cornelia Huck, qemu-trivial

Le 26/10/2022 à 01:50, Philippe Mathieu-Daudé a écrit :
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/m68k/{qregs.def => qregs.h.inc} | 0
>   target/m68k/translate.c                | 4 ++--
>   2 files changed, 2 insertions(+), 2 deletions(-)
>   rename target/m68k/{qregs.def => qregs.h.inc} (100%)
> 
> diff --git a/target/m68k/qregs.def b/target/m68k/qregs.h.inc
> similarity index 100%
> rename from target/m68k/qregs.def
> rename to target/m68k/qregs.h.inc
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 9df17aa4b2..f018fa9eb0 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -39,7 +39,7 @@
>   
>   #define DEFO32(name, offset) static TCGv QREG_##name;
>   #define DEFO64(name, offset) static TCGv_i64 QREG_##name;
> -#include "qregs.def"
> +#include "qregs.h.inc"
>   #undef DEFO32
>   #undef DEFO64
>   
> @@ -75,7 +75,7 @@ void m68k_tcg_init(void)
>   #define DEFO64(name, offset) \
>       QREG_##name = tcg_global_mem_new_i64(cpu_env, \
>           offsetof(CPUM68KState, offset), #name);
> -#include "qregs.def"
> +#include "qregs.h.inc"
>   #undef DEFO32
>   #undef DEFO64
>   

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc
  2022-10-25 23:50 ` [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc Philippe Mathieu-Daudé
@ 2022-10-27  6:45   ` Thomas Huth
  2022-11-02 18:30   ` Laurent Vivier
  1 sibling, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2022-10-27  6:45 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, qemu-s390x,
	Richard Henderson, Laurent Vivier, Cornelia Huck, qemu-trivial

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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   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 +++++-----
>   3 files changed, 6 insertions(+), 6 deletions(-)
>   rename target/s390x/tcg/{insn-data.def => insn-data.h.inc} (99%)
>   rename target/s390x/tcg/{insn-format.def => insn-format.h.inc} (100%)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
       [not found] ` <8d197b71-937a-5693-3b7f-ea4bded8c360@redhat.com>
@ 2022-10-27 14:12   ` Philippe Mathieu-Daudé
  2022-10-27 15:25     ` Thomas Huth
  0 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-27 14:12 UTC (permalink / raw)
  To: Thomas Huth; +Cc: QEMU Developers, Paolo Bonzini, Daniel P. Berrangé

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.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
  2022-10-25 23:50 [PATCH 0/3] target: Rename headers using .def extension to .h.inc Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
       [not found] ` <8d197b71-937a-5693-3b7f-ea4bded8c360@redhat.com>
@ 2022-10-27 14:39 ` Markus Armbruster
  2022-10-27 15:01   ` Peter Maydell
  5 siblings, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2022-10-27 14:39 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, David Hildenbrand, Bastian Koppelmann, Thomas Huth,
	qemu-s390x, Richard Henderson, Laurent Vivier, Cornelia Huck,
	qemu-trivial

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.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
  2022-10-27 14:39 ` Markus Armbruster
@ 2022-10-27 15:01   ` Peter Maydell
  2022-10-27 17:17     ` Markus Armbruster
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2022-10-27 15:01 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Philippe Mathieu-Daudé,
	qemu-devel, David Hildenbrand, Bastian Koppelmann, Thomas Huth,
	qemu-s390x, Richard Henderson, Laurent Vivier, Cornelia Huck,
	qemu-trivial

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
  2022-10-27 14:12   ` Philippe Mathieu-Daudé
@ 2022-10-27 15:25     ` Thomas Huth
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2022-10-27 15:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: QEMU Developers, Paolo Bonzini, Daniel P. Berrangé

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



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
  2022-10-27 15:01   ` Peter Maydell
@ 2022-10-27 17:17     ` Markus Armbruster
  2022-10-27 17:24       ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2022-10-27 17:17 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Philippe Mathieu-Daudé,
	qemu-devel, David Hildenbrand, Bastian Koppelmann, Thomas Huth,
	qemu-s390x, Richard Henderson, Laurent Vivier, Cornelia Huck,
	qemu-trivial

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.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
  2022-10-27 17:17     ` Markus Armbruster
@ 2022-10-27 17:24       ` Peter Maydell
  2022-10-28  4:37         ` Markus Armbruster
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2022-10-27 17:24 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Philippe Mathieu-Daudé,
	qemu-devel, David Hildenbrand, Bastian Koppelmann, Thomas Huth,
	qemu-s390x, Richard Henderson, Laurent Vivier, Cornelia Huck,
	qemu-trivial

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] target: Rename headers using .def extension to .h.inc
  2022-10-27 17:24       ` Peter Maydell
@ 2022-10-28  4:37         ` Markus Armbruster
  0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2022-10-28  4:37 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Philippe Mathieu-Daudé,
	qemu-devel, David Hildenbrand, Bastian Koppelmann, Thomas Huth,
	qemu-s390x, Richard Henderson, Laurent Vivier, Cornelia Huck,
	qemu-trivial

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 :)



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc
  2022-10-25 23:50 ` [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc Philippe Mathieu-Daudé
  2022-10-26  9:54   ` Laurent Vivier
@ 2022-11-02 18:29   ` Laurent Vivier
  1 sibling, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2022-11-02 18:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Cornelia Huck, qemu-trivial

Le 26/10/2022 à 01:50, Philippe Mathieu-Daudé a écrit :
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/m68k/{qregs.def => qregs.h.inc} | 0
>   target/m68k/translate.c                | 4 ++--
>   2 files changed, 2 insertions(+), 2 deletions(-)
>   rename target/m68k/{qregs.def => qregs.h.inc} (100%)
> 
> diff --git a/target/m68k/qregs.def b/target/m68k/qregs.h.inc
> similarity index 100%
> rename from target/m68k/qregs.def
> rename to target/m68k/qregs.h.inc
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 9df17aa4b2..f018fa9eb0 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -39,7 +39,7 @@
>   
>   #define DEFO32(name, offset) static TCGv QREG_##name;
>   #define DEFO64(name, offset) static TCGv_i64 QREG_##name;
> -#include "qregs.def"
> +#include "qregs.h.inc"
>   #undef DEFO32
>   #undef DEFO64
>   
> @@ -75,7 +75,7 @@ void m68k_tcg_init(void)
>   #define DEFO64(name, offset) \
>       QREG_##name = tcg_global_mem_new_i64(cpu_env, \
>           offsetof(CPUM68KState, offset), #name);
> -#include "qregs.def"
> +#include "qregs.h.inc"
>   #undef DEFO32
>   #undef DEFO64
>   

Applied to my trivial-patches branch.

Thanks,
Laurent




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc
  2022-10-25 23:50 ` [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc Philippe Mathieu-Daudé
  2022-10-27  6:45   ` Thomas Huth
@ 2022-11-02 18:30   ` Laurent Vivier
  1 sibling, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2022-11-02 18:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Cornelia Huck, qemu-trivial

Le 26/10/2022 à 01:50, Philippe Mathieu-Daudé a écrit :
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   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 +++++-----
>   3 files changed, 6 insertions(+), 6 deletions(-)
>   rename target/s390x/tcg/{insn-data.def => insn-data.h.inc} (99%)
>   rename target/s390x/tcg/{insn-format.def => insn-format.h.inc} (100%)
> 
> diff --git a/target/s390x/tcg/insn-data.def b/target/s390x/tcg/insn-data.h.inc
> similarity index 99%
> rename from target/s390x/tcg/insn-data.def
> rename to target/s390x/tcg/insn-data.h.inc
> index 6382ceabfc..7e952bdfc8 100644
> --- a/target/s390x/tcg/insn-data.def
> +++ b/target/s390x/tcg/insn-data.h.inc
> @@ -8,7 +8,7 @@
>    *
>    *  OPC  = (op << 8) | op2 where op is the major, op2 the minor opcode
>    *  NAME = name of the opcode, used internally
> - *  FMT  = format of the opcode (defined in insn-format.def)
> + *  FMT  = format of the opcode (defined in insn-format.h.inc)
>    *  FAC  = facility the opcode is available in (defined in DisasFacility)
>    *  I1   = func in1_xx fills o->in1
>    *  I2   = func in2_xx fills o->in2
> diff --git a/target/s390x/tcg/insn-format.def b/target/s390x/tcg/insn-format.h.inc
> similarity index 100%
> rename from target/s390x/tcg/insn-format.def
> rename to target/s390x/tcg/insn-format.h.inc
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 1d2dddab1c..f378e1a633 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -1011,7 +1011,7 @@ static void free_compare(DisasCompare *c)
>   #define F6(N, X1, X2, X3, X4, X5, X6) F0(N)
>   
>   typedef enum {
> -#include "insn-format.def"
> +#include "insn-format.h.inc"
>   } DisasFormat;
>   
>   #undef F0
> @@ -1076,7 +1076,7 @@ typedef struct DisasFormatInfo {
>   #define F6(N, X1, X2, X3, X4, X5, X6)       { { X1, X2, X3, X4, X5, X6 } },
>   
>   static const DisasFormatInfo format_info[] = {
> -#include "insn-format.def"
> +#include "insn-format.h.inc"
>   };
>   
>   #undef F0
> @@ -6143,7 +6143,7 @@ static void in2_insn(DisasContext *s, DisasOps *o)
>   #define E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D, FL) insn_ ## NM,
>   
>   enum DisasInsnEnum {
> -#include "insn-data.def"
> +#include "insn-data.h.inc"
>   };
>   
>   #undef E
> @@ -6223,7 +6223,7 @@ enum DisasInsnEnum {
>   #define FAC_MIE3        S390_FEAT_MISC_INSTRUCTION_EXT3 /* miscellaneous-instruction-extensions facility 3 */
>   
>   static const DisasInsn insn_info[] = {
> -#include "insn-data.def"
> +#include "insn-data.h.inc"
>   };
>   
>   #undef E
> @@ -6233,7 +6233,7 @@ static const DisasInsn insn_info[] = {
>   static const DisasInsn *lookup_opc(uint16_t opc)
>   {
>       switch (opc) {
> -#include "insn-data.def"
> +#include "insn-data.h.inc"
>       default:
>           return NULL;
>       }

Applied to my trivial-patches branch.

Thanks,
Laurent




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc
  2022-10-25 23:50 ` [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc Philippe Mathieu-Daudé
  2022-10-26  8:21   ` Bastian Koppelmann
@ 2022-11-02 18:30   ` Laurent Vivier
  1 sibling, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2022-11-02 18:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: David Hildenbrand, Bastian Koppelmann, Thomas Huth, qemu-s390x,
	Richard Henderson, Cornelia Huck, qemu-trivial

Le 26/10/2022 à 01:50, Philippe Mathieu-Daudé a écrit :
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/tricore/{csfr.def => csfr.h.inc} | 0
>   target/tricore/translate.c              | 4 ++--
>   2 files changed, 2 insertions(+), 2 deletions(-)
>   rename target/tricore/{csfr.def => csfr.h.inc} (100%)
> 
> diff --git a/target/tricore/csfr.def b/target/tricore/csfr.h.inc
> similarity index 100%
> rename from target/tricore/csfr.def
> rename to target/tricore/csfr.h.inc
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> index a0558ead71..f02090945d 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -388,7 +388,7 @@ static inline void gen_mfcr(DisasContext *ctx, TCGv ret, int32_t offset)
>           gen_helper_psw_read(ret, cpu_env);
>       } else {
>           switch (offset) {
> -#include "csfr.def"
> +#include "csfr.h.inc"
>           }
>       }
>   }
> @@ -418,7 +418,7 @@ static inline void gen_mtcr(DisasContext *ctx, TCGv r1,
>               gen_helper_psw_write(cpu_env, r1);
>           } else {
>               switch (offset) {
> -#include "csfr.def"
> +#include "csfr.h.inc"
>               }
>           }
>       } else {

Applied to my trivial-patches branch.

Thanks,
Laurent




^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-11-02 18:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-25 23:50 [PATCH 0/3] target: Rename headers using .def extension to .h.inc Philippe Mathieu-Daudé
2022-10-25 23:50 ` [PATCH 1/3] target/m68k: Rename qregs.def -> qregs.h.inc Philippe Mathieu-Daudé
2022-10-26  9:54   ` Laurent Vivier
2022-11-02 18:29   ` Laurent Vivier
2022-10-25 23:50 ` [PATCH 2/3] target/s390x: Rename insn-data/format.def -> insn-data/format.h.inc Philippe Mathieu-Daudé
2022-10-27  6:45   ` Thomas Huth
2022-11-02 18:30   ` Laurent Vivier
2022-10-25 23:50 ` [PATCH 3/3] target/tricore: Rename csfr.def -> csfr.h.inc Philippe Mathieu-Daudé
2022-10-26  8:21   ` Bastian Koppelmann
2022-11-02 18:30   ` Laurent Vivier
2022-10-26  9:45 ` [PATCH 0/3] target: Rename headers using .def extension to .h.inc Alex Bennée
     [not found] ` <8d197b71-937a-5693-3b7f-ea4bded8c360@redhat.com>
2022-10-27 14:12   ` Philippe Mathieu-Daudé
2022-10-27 15:25     ` Thomas Huth
2022-10-27 14:39 ` Markus Armbruster
2022-10-27 15:01   ` Peter Maydell
2022-10-27 17:17     ` Markus Armbruster
2022-10-27 17:24       ` Peter Maydell
2022-10-28  4:37         ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.