[PATCH] D69471: [Coverage] Revise format to reduce binary size

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 20 14:20:51 PST 2020


rnk added a comment.

In D69471#1884043 <https://reviews.llvm.org/D69471#1884043>, @dexonsmith wrote:

> In D69471#1883912 <https://reviews.llvm.org/D69471#1883912>, @rnk wrote:
>
> > Everything is off-by-one because the empty bases are not zero sized. The MSVC record layout algorithm is just different in this area. =/
>
>
> Do all the MSVCs we support building with support `__declspec(empty_bases)`?
>  https://devblogs.microsoft.com/cppblog/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/


Yes, we can use it. Clang supports it too.

---

However, you might want this class to follow the rules for a standard layout type:
http://www.cplusplus.com/reference/type_traits/is_standard_layout/
" ...  has .... at most one base class with non-static data members, or has no base classes with non-static data members."
So the use of multiple inheritance makes a type non-standard layout. Nevermind that MSVC accepts the following program:

  #include <type_traits>
  struct EmptyA {};
  struct EmptyB {};
  struct Foo : EmptyA, EmptyB { int x, y; };
  static_assert(std::is_standard_layout<Foo>::value, "asdf");

Why not use free function templates like these?

  template <typename T> uint64_t getFoo(T *p) { return p->Foo; }
  template <typename T> uint64_t getBar(T *p) { return p->Bar; }

These accessors don't seem like they have to be methods. It actually makes them a bit more awkward to use:

  uint32_t DataSize = CFR->template getDataSize<Endian>();


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69471/new/

https://reviews.llvm.org/D69471





More information about the cfe-commits mailing list