Matt Turner <matts...@gmail.com> writes: > On Tue, Nov 3, 2015 at 5:48 AM, Francisco Jerez <curroje...@riseup.net> wrote: >> Matt Turner <matts...@gmail.com> writes: >> >>> Generated by >>> >>> sed -i -e 's/\.bits\././g' *.c *.h *.cpp >>> sed -i -e 's/dw1\.//g' *.c *.h *.cpp >>> >>> and then reverting changes to comments in gen7_blorp.cpp and >>> brw_fs_generator.cpp. >>> >>> There wasn't any utility offered by forcing the programmer to list these >>> to access their fields. Removing them will reduce churn in future >>> commits. >>> >>> This is C11 (and gcc has apparently supported it for sometime >>> "compatibility with other compilers") >>> >>> See https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html >> >> This is also used from C++ source where anonymous structs are not part >> of any released standard. > > That is true. I have built this series with both clang-3.6 and > gcc-4.4.7. I don't think it's a problem. > >> I guess in C++ it would be preferable to >> define accessor methods instead of relying on a language extension -- >> That would also allow you to introduce checks making sure that the >> register is of the correct type in order to catch cases in which the >> wrong field of the union is accessed easily. > > Maybe. Since I was changing so much code in this series, I wouldn't > want to do that here.
Yeah, I agree that there's no need to make that change as part of this series. However if we agree that accessors would be a better approach this patch is unnecessary, since we will get a similar kind of syntactic sugar without relying on unnamed structures. > Also, having commits that use the brw_reg fields separately from any > accessors seems beneficial. > > We could also simply mark fields private with using declarations. That > would get almost all of any potential benefit (of which I'm not sure > how much there is, really).
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
