On Apr 19, 2016 6:10 PM, "Emil Velikov" <emil.l.veli...@gmail.com> wrote:
>
> On 19 April 2016 at 23:18, Chad Versace <chad.vers...@intel.com> wrote:
> > On Tue 19 Apr 2016, Emil Velikov wrote:
> >> On 16 April 2016 at 20:45, Jason Ekstrand <ja...@jlekstrand.net> 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] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
