[PATCH] Fixing a struct layout bug with bitfields before virtual base

Yunzhong Gao Yunzhong_Gao at playstation.sony.com
Sat Jan 25 00:09:47 PST 2014



================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:347
@@ -346,2 +346,3 @@
   // beneficial.
   uint64_t EndOffset = Types.getContext().toBits(Layout.getDataSize());
+  if (Types.getContext().getLangOpts().CPlusPlus)
----------------
David Majnemer wrote:
> This would make more sense written as:
> 
>   uint64_t EndOffset;
>   if (Types.getContext().getLangOpts().CPlusPlus)
>     EndOffset = Types.getContext().toBits(Layout.getNonVirtualSize());
>   else
>     EndOffset = Types.getContext().toBits(Layout.getDataSize());
> 
> That being said, why do we need the language check at all? Couldn't we always use `Layout.getNonVirtualSize()`?
Yes I can change that. Or do you think it will be even better to use a ?: operator:
```
uint64_t EndOffset = Types.getContext().toBits(
  Types.getContext().getLangOpts().CPlusPlus ?
  Layout.getNonVirtualSize() : Layout.getDataSize());
```

The language check is there to avoid an assertion failure in getNonVirtualSize().
getNonVirtualSize() queries CXXInfo->NonVirtualSize, and the member CXXInfo
is null for non-C++ records. There are two constructors of ASTRecordLayout
for C++ and non-C++ records respectively.


http://llvm-reviews.chandlerc.com/D2560



More information about the cfe-commits mailing list