[PATCH] xtensa: Avoid designated inits, for C++ compliance

[PATCH] xtensa: Avoid designated inits, for C++ compliance

[Andreas Arnez]

C++ does not officially support designators in initializer lists.  Thus
some compilers may issue errors when encountering them.  Modern versions
of GCC seem to allow them by default, as a GCC extension, even though
the GCC documentation explicitly states otherwise: "[...] This extension
is not implemented in GNU C++."  But some older GCC versions (like
4.4.7) did indeed emit an error instead, like this:

  .../gdb/xtensa-config.c:219: error: expected primary-expression before
			       ‘.’ token

This patch removes the only such instance I've seen when building with
'--enable-targets=all'.

gdb/ChangeLog:

	* xtensa-tdep.h (XTENSA_GDBARCH_TDEP_INSTANTIATE): Replace
	designated initializer list by plain initializer list, for C++
	compliance.
---
 gdb/xtensa-tdep.h | 98 +++++++++++++++++++++++++++----------------------------
 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/gdb/xtensa-tdep.h b/gdb/xtensa-tdep.h
index b2b1903..c83e312 100644
--- a/gdb/xtensa-tdep.h
+++ b/gdb/xtensa-tdep.h
@@ -224,55 +224,55 @@ struct gdbarch_tdep
 
 /* Macro to instantiate a gdbarch_tdep structure.  */
 
-#define XTENSA_GDBARCH_TDEP_INSTANTIATE(rmap,spillsz)		\
-	{							\
-	  .target_flags = 0,					\
-	  .spill_location = -1,					\
-	  .spill_size = (spillsz),				\
-	  .unused = 0,						\
-	  .call_abi = (XSHAL_ABI == XTHAL_ABI_CALL0		\
-		       ? CallAbiCall0Only			\
-		       : CallAbiDefault),			\
-	  .debug_interrupt_level = XCHAL_DEBUGLEVEL,		\
-	  .icache_line_bytes = XCHAL_ICACHE_LINESIZE,		\
-	  .dcache_line_bytes = XCHAL_DCACHE_LINESIZE,		\
-	  .dcache_writeback = XCHAL_DCACHE_IS_WRITEBACK,	\
-	  .isa_use_windowed_registers = (XSHAL_ABI != XTHAL_ABI_CALL0),	\
-	  .isa_use_density_instructions = XCHAL_HAVE_DENSITY,	\
-	  .isa_use_exceptions = XCHAL_HAVE_EXCEPTIONS,		\
-	  .isa_use_ext_l32r = XSHAL_USE_ABSOLUTE_LITERALS,	\
-	  .isa_max_insn_size = XCHAL_MAX_INSTRUCTION_SIZE,	\
-	  .debug_num_ibreaks = XCHAL_NUM_IBREAK,		\
-	  .debug_num_dbreaks = XCHAL_NUM_DBREAK,		\
-	  .regmap = rmap,			\
-	  .num_regs = 0,			\
-	  .num_nopriv_regs = 0,			\
-	  .num_pseudo_regs = 0,			\
-	  .num_aregs = XCHAL_NUM_AREGS,		\
-	  .num_contexts = XCHAL_NUM_CONTEXTS,	\
-	  .ar_base = -1,			\
-	  .a0_base = -1,			\
-	  .wb_regnum = -1,			\
-	  .ws_regnum = -1,			\
-	  .pc_regnum = -1,			\
-	  .ps_regnum = -1,			\
-	  .lbeg_regnum = -1,			\
-	  .lend_regnum = -1,			\
-	  .lcount_regnum = -1,			\
-	  .sar_regnum = -1,			\
-	  .litbase_regnum = -1,			\
-	  .interrupt_regnum = -1,		\
-	  .interrupt2_regnum = -1,		\
-	  .cpenable_regnum = -1,		\
-	  .debugcause_regnum = -1,		\
-	  .exccause_regnum = -1,		\
-	  .excvaddr_regnum = -1,		\
-	  .max_register_raw_size = 0,		\
-	  .max_register_virtual_size = 0,	\
-	  .fp_layout = 0,			\
-	  .fp_layout_bytes = 0,			\
-	  .gregmap = 0,				\
-	}
+#define XTENSA_GDBARCH_TDEP_INSTANTIATE(rmap,spillsz)			\
+  {									\
+    0,				/* target_flags */			\
+    -1,				/* spill_location */			\
+    (spillsz),			/* spill_size */			\
+    0,				/* unused */				\
+    (XSHAL_ABI == XTHAL_ABI_CALL0					\
+     ? CallAbiCall0Only							\
+     : CallAbiDefault),		/* call_abi */				\
+    XCHAL_DEBUGLEVEL,		/* debug_interrupt_level */		\
+    XCHAL_ICACHE_LINESIZE,	/* icache_line_bytes */			\
+    XCHAL_DCACHE_LINESIZE,	/* dcache_line_bytes */			\
+    XCHAL_DCACHE_IS_WRITEBACK,  /* dcache_writeback */			\
+    (XSHAL_ABI != XTHAL_ABI_CALL0),   /* isa_use_windowed_registers */	\
+    XCHAL_HAVE_DENSITY,		 /* isa_use_density_instructions */	\
+    XCHAL_HAVE_EXCEPTIONS,	 /* isa_use_exceptions */		\
+    XSHAL_USE_ABSOLUTE_LITERALS, /* isa_use_ext_l32r */			\
+    XCHAL_MAX_INSTRUCTION_SIZE,  /* isa_max_insn_size */		\
+    XCHAL_NUM_IBREAK,		 /* debug_num_ibreaks */		\
+    XCHAL_NUM_DBREAK,		 /* debug_num_dbreaks */		\
+    rmap,			 /* regmap */				\
+    0,				 /* num_regs */				\
+    0,				 /* num_nopriv_regs */			\
+    0,				 /* num_pseudo_regs */			\
+    XCHAL_NUM_AREGS,		 /* num_aregs */			\
+    XCHAL_NUM_CONTEXTS,		 /* num_contexts */			\
+    -1,				 /* ar_base */				\
+    -1,				 /* a0_base */				\
+    -1,				 /* wb_regnum */			\
+    -1,				 /* ws_regnum */			\
+    -1,				 /* pc_regnum */			\
+    -1,				 /* ps_regnum */			\
+    -1,				 /* lbeg_regnum */			\
+    -1,				 /* lend_regnum */			\
+    -1,				 /* lcount_regnum */			\
+    -1,				 /* sar_regnum */			\
+    -1,				 /* litbase_regnum */			\
+    -1,				 /* interrupt_regnum */			\
+    -1,				 /* interrupt2_regnum */		\
+    -1,				 /* cpenable_regnum */			\
+    -1,				 /* debugcause_regnum */		\
+    -1,				 /* exccause_regnum */			\
+    -1,				 /* excvaddr_regnum */			\
+    0,				 /* max_register_raw_size */		\
+    0,				 /* max_register_virtual_size */	\
+    0,				 /* fp_layout */			\
+    0,				 /* fp_layout_bytes */			\
+    0,				 /* gregmap */				\
+  }
 #define XTENSA_CONFIG_INSTANTIATE(rmap,spill_size)	\
 	struct gdbarch_tdep xtensa_tdep = \
 	  XTENSA_GDBARCH_TDEP_INSTANTIATE(rmap,spill_size);
-- 
2.5.0

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Pedro Alves]

OK.

Thanks,
Pedro Alves

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Andreas Arnez]

On Thu, Aug 25 2016, Pedro Alves wrote:

> OK.
>
> Thanks,
> Pedro Alves

Thanks, pushed.

--
Andreas

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

On Thu, 25 Aug 2016 13:56:25 +0200, Andreas Arnez wrote:
> C++ does not officially support designators in initializer lists.

I could miss something about the transition to C++ but C++ was not intended to
make the sources worse, it should improve the sources.  The checked in change
makes the source apparently more error prone to future changes.  Why not the
attached?

Besides that is only one step; the ctor should be constexpr I guess, but that
is C++11 again.

I have noticed now the C++ conversion does not seem to be complete to me
- struct gdbarch_tdep is defined differently for different translation units,
this violates:
	https://en.wikipedia.org/wiki/One_Definition_Rule


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 4303 bytes --]

diff --git a/gdb/xtensa-config.c b/gdb/xtensa-config.c
index 0773eda..99da40f 100644
--- a/gdb/xtensa-config.c
+++ b/gdb/xtensa-config.c
@@ -215,7 +215,54 @@ xtensa_register_t rmap[] =
 
 
 
-#ifdef XTENSA_CONFIG_INSTANTIATE
-XTENSA_CONFIG_INSTANTIATE(rmap,0)
-#endif
-
+class XtensaTdep:public gdbarch_tdep {
+public:
+  XtensaTdep() {
+    target_flags = 0;
+    spill_location = -1;
+    spill_size = 0;
+    unused = 0;
+    call_abi = (XSHAL_ABI == XTHAL_ABI_CALL0
+		? CallAbiCall0Only
+		: CallAbiDefault);
+    debug_interrupt_level = XCHAL_DEBUGLEVEL;
+    icache_line_bytes = XCHAL_ICACHE_LINESIZE;
+    dcache_line_bytes = XCHAL_DCACHE_LINESIZE;
+    dcache_writeback = XCHAL_DCACHE_IS_WRITEBACK;
+    isa_use_windowed_registers = (XSHAL_ABI != XTHAL_ABI_CALL0);
+    isa_use_density_instructions = XCHAL_HAVE_DENSITY;
+    isa_use_exceptions = XCHAL_HAVE_EXCEPTIONS;
+    isa_use_ext_l32r = XSHAL_USE_ABSOLUTE_LITERALS;
+    isa_max_insn_size = XCHAL_MAX_INSTRUCTION_SIZE;
+    debug_num_ibreaks = XCHAL_NUM_IBREAK;
+    debug_num_dbreaks = XCHAL_NUM_DBREAK;
+    regmap = rmap;
+    num_regs = 0;
+    num_nopriv_regs = 0;
+    num_pseudo_regs = 0;
+    num_aregs = XCHAL_NUM_AREGS;
+    num_contexts = XCHAL_NUM_CONTEXTS;
+    ar_base = -1;
+    a0_base = -1;
+    wb_regnum = -1;
+    ws_regnum = -1;
+    pc_regnum = -1;
+    ps_regnum = -1;
+    lbeg_regnum = -1;
+    lend_regnum = -1;
+    lcount_regnum = -1;
+    sar_regnum = -1;
+    litbase_regnum = -1;
+    interrupt_regnum = -1;
+    interrupt2_regnum = -1;
+    cpenable_regnum = -1;
+    debugcause_regnum = -1;
+    exccause_regnum = -1;
+    excvaddr_regnum = -1;
+    max_register_raw_size = 0;
+    max_register_virtual_size = 0;
+    fp_layout = 0;
+    fp_layout_bytes = 0;
+    gregmap = 0;
+  }
+} xtensa_tdep;
diff --git a/gdb/xtensa-tdep.h b/gdb/xtensa-tdep.h
index c83e312..1ddfb73 100644
--- a/gdb/xtensa-tdep.h
+++ b/gdb/xtensa-tdep.h
@@ -222,61 +222,6 @@ struct gdbarch_tdep
   struct ctype_cache *type_entries;
 };
 
-/* Macro to instantiate a gdbarch_tdep structure.  */
-
-#define XTENSA_GDBARCH_TDEP_INSTANTIATE(rmap,spillsz)			\
-  {									\
-    0,				/* target_flags */			\
-    -1,				/* spill_location */			\
-    (spillsz),			/* spill_size */			\
-    0,				/* unused */				\
-    (XSHAL_ABI == XTHAL_ABI_CALL0					\
-     ? CallAbiCall0Only							\
-     : CallAbiDefault),		/* call_abi */				\
-    XCHAL_DEBUGLEVEL,		/* debug_interrupt_level */		\
-    XCHAL_ICACHE_LINESIZE,	/* icache_line_bytes */			\
-    XCHAL_DCACHE_LINESIZE,	/* dcache_line_bytes */			\
-    XCHAL_DCACHE_IS_WRITEBACK,  /* dcache_writeback */			\
-    (XSHAL_ABI != XTHAL_ABI_CALL0),   /* isa_use_windowed_registers */	\
-    XCHAL_HAVE_DENSITY,		 /* isa_use_density_instructions */	\
-    XCHAL_HAVE_EXCEPTIONS,	 /* isa_use_exceptions */		\
-    XSHAL_USE_ABSOLUTE_LITERALS, /* isa_use_ext_l32r */			\
-    XCHAL_MAX_INSTRUCTION_SIZE,  /* isa_max_insn_size */		\
-    XCHAL_NUM_IBREAK,		 /* debug_num_ibreaks */		\
-    XCHAL_NUM_DBREAK,		 /* debug_num_dbreaks */		\
-    rmap,			 /* regmap */				\
-    0,				 /* num_regs */				\
-    0,				 /* num_nopriv_regs */			\
-    0,				 /* num_pseudo_regs */			\
-    XCHAL_NUM_AREGS,		 /* num_aregs */			\
-    XCHAL_NUM_CONTEXTS,		 /* num_contexts */			\
-    -1,				 /* ar_base */				\
-    -1,				 /* a0_base */				\
-    -1,				 /* wb_regnum */			\
-    -1,				 /* ws_regnum */			\
-    -1,				 /* pc_regnum */			\
-    -1,				 /* ps_regnum */			\
-    -1,				 /* lbeg_regnum */			\
-    -1,				 /* lend_regnum */			\
-    -1,				 /* lcount_regnum */			\
-    -1,				 /* sar_regnum */			\
-    -1,				 /* litbase_regnum */			\
-    -1,				 /* interrupt_regnum */			\
-    -1,				 /* interrupt2_regnum */		\
-    -1,				 /* cpenable_regnum */			\
-    -1,				 /* debugcause_regnum */		\
-    -1,				 /* exccause_regnum */			\
-    -1,				 /* excvaddr_regnum */			\
-    0,				 /* max_register_raw_size */		\
-    0,				 /* max_register_virtual_size */	\
-    0,				 /* fp_layout */			\
-    0,				 /* fp_layout_bytes */			\
-    0,				 /* gregmap */				\
-  }
-#define XTENSA_CONFIG_INSTANTIATE(rmap,spill_size)	\
-	struct gdbarch_tdep xtensa_tdep = \
-	  XTENSA_GDBARCH_TDEP_INSTANTIATE(rmap,spill_size);
-
 #ifndef XCHAL_NUM_CONTEXTS
 #define XCHAL_NUM_CONTEXTS	0
 #endif

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Eli Zaretskii]

> Date: Fri, 26 Aug 2016 11:01:48 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org, Pedro Alves <palves@redhat.com>
> 
> 
> [1:text/plain Hide]
> 
> On Thu, 25 Aug 2016 13:56:25 +0200, Andreas Arnez wrote:
> > C++ does not officially support designators in initializer lists.
> 
> I could miss something about the transition to C++ but C++ was not intended to
> make the sources worse, it should improve the sources.  The checked in change
> makes the source apparently more error prone to future changes.  Why not the
> attached?

Because it's C++, not C?

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Pedro Alves]

On 08/26/2016 10:01 AM, Jan Kratochvil wrote:
> On Thu, 25 Aug 2016 13:56:25 +0200, Andreas Arnez wrote:
>> C++ does not officially support designators in initializer lists.
> 
> I could miss something about the transition to C++ (...)

You're missing that we haven't dropped support for building with
a C compiler yet...

We're still doing many 7.12 backports, so I was waiting for that
to stabilize before proposing to drop C support.

> The checked in change
> makes the source apparently more error prone to future changes.  Why not the
> attached?

Note that this header is meant to be replaced as part of an "overlay".
See:

 http://wiki.linux-xtensa.org/index.php/Toolchain_Overlay_File
 https://www.sourceware.org/ml/gdb-patches/2015-04/msg00695.html

> I have noticed now the C++ conversion does not seem to be complete to me
> - struct gdbarch_tdep is defined differently for different translation units,
> this violates:
> 	https://en.wikipedia.org/wiki/One_Definition_Rule

Right, we're not LTO-ready yet.  Note -Wodr (w/ -flto) helps find these.

Thanks,
Pedro Alves

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Pedro Alves]

On 08/25/2016 06:17 PM, Andreas Arnez wrote:
> On Thu, Aug 25 2016, Pedro Alves wrote:
> 
>> OK.
>>
>> Thanks,
>> Pedro Alves
> 
> Thanks, pushed.

I think it'd be good to put this in the 7.12 branch as well.
Could you do that?

Thanks,
Pedro Alves

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Jan Kratochvil]

On Fri, 26 Aug 2016 11:39:27 +0200, Pedro Alves wrote:
> We're still doing many 7.12 backports, so I was waiting for that
> to stabilize before proposing to drop C support.

Ah, OK.


> > 	https://en.wikipedia.org/wiki/One_Definition_Rule
> 
> Right, we're not LTO-ready yet.  Note -Wodr (w/ -flto) helps find these.

In this case there would be gdbarch_tdep() ctor handy but that is also not
applicable without ODR.  OK, I see I missed GDB still does not require C++.


Jan

Re: [PATCH] xtensa: Avoid designated inits, for C++ compliance

[Andreas Arnez]

On Fri, Aug 26 2016, Pedro Alves wrote:

> On 08/25/2016 06:17 PM, Andreas Arnez wrote:
>> On Thu, Aug 25 2016, Pedro Alves wrote:
>> 
>>> OK.
>>>
>>> Thanks,
>>> Pedro Alves
>> 
>> Thanks, pushed.
>
> I think it'd be good to put this in the 7.12 branch as well.
> Could you do that?

Sure, done.

--
Andreas