On Apr 19, 2016 6:10 PM, "Emil Velikov" <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> wrote: > > On 19 April 2016 at 23:18, Chad Versace <<a href="mailto:chad.versace@intel.com">chad.versace@intel.com</a>> wrote: > > On Tue 19 Apr 2016, Emil Velikov wrote: > >> On 16 April 2016 at 20:45, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > >> > C++ doesn't support designated initializers and g++ in particular doesn't > >> > handle them when the struct gets complicated, i.e. has a union. > >> > --- > >> > src/intel/isl/isl.h | 32 ++++++++++++++++++++------------ > >> > 1 file changed, 20 insertions(+), 12 deletions(-) > >> > > >> > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h > >> > index 33d43d7..c3a1106 100644 > >> > --- a/src/intel/isl/isl.h > >> > +++ b/src/intel/isl/isl.h > >> > @@ -988,25 +988,35 @@ isl_surf_info_is_z32_float(const struct isl_surf_init_info *info) > >> > static inline struct isl_extent2d > >> > isl_extent2d(uint32_t width, uint32_t height) > >> > { > >> > - return (struct isl_extent2d) { .w = width, .h = height }; > >> > + struct isl_extent2d e = { > >> > + { width, }, > >> > + { height, }, > >> > + }; > >> > + return e; > >> > } > >> > > >> Please use something like the following > >> > >> struct isl_extent2d e; > >> > >> memset(&e, 0, sizeof(e)); > >> e.w = width; > >> e.h = height; > >> > >> return e; > >> > >> It is a bit over expressive to write/look at, although it will give > >> you exactly what you want _all_ the time. > >> Otherwise you risk feeding garbage and/or setting the wrong struct > >> member as isl_extent2d change. > >> > >> Hunting bugs that this may cause, is not what you want to do. Plus the > >> compiler will optimise it to the exact same code. > > > > I agree with Emil. Let's use explicit member names when initializing the > > structs. But, the memset isn't needed. The below initializaing pattern > > is more concise and *safer*, as it doesn't requiring typing the struct > > size. > > > > struct isl_extent2d e = {0}; > > > Almost... this won't play nice with C++ contexts. There's also the > fact that some C compilers will bark at us if we nest something within > the isl_extend2d struct, despite that the c99 standard (or was it c89) > is pretty clear that the above is sufficient. > > Namely it will complain that we need another set of brackets. Check > out the GCC bug [1] about this. I added more brackets. :-) > So yes, it's annoyingly long/verbose solution but for a reason ;-) > -Emil > > [1] <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119">https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119</a>