[PATCH] D158145: [clang] Update NumFunctionDeclBits for FunctionDeclBitfields

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D158145#4594549 <https://reviews.llvm.org/D158145#4594549>, @danix800 wrote:

> I also investigated whether we could count those bits at compile time and statically assert on them,
> because a small typo or missed update could spend us a lot of time to dig for the cause.
>
> My first step is trying to count number of bits for a single bitfield, this is promising based on this <https://gist.github.com/RMDarth/6357ddd6e09b4117efe84fc3347a732a> but
> with a restriction, it only works on `struct` (default public fields), not `class` (default to private fields).
>
> If we can implement this `bitsizeof` then we could have:
>
>   enum { NumFunctionDeclBits = offsetof(FunctionDeclBitfields, SClass)
>                              + offsetof(FunctionDeclBitfields, IsInline)
>                              + ... };
>
> This can automatically update total number of bits if any of the existing one is updated.
>
> The second step is trying to enumerate all bit fields at compile time so that we can totally fix this kind
> of issue, but it seems not possible.
>
> Any suggestions or advices? Is it even worth it to do it like this?

It would be great to be able to stattic_assert these sort of bugs. I think @aaron did some research in this area, it might be worth asking him how far he got!



================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:7836
+TEST_P(ASTImporterOptionSpecificTestBase,
+    ImportFunctionDeclBitShouldNotStampingOnCtorDeclBits) {
+  Decl *From, *To;
----------------
danix800 wrote:
> cor3ntin wrote:
> > `ImportFunctionDeclBitShouldNotOverwriteCtorDeclBits`
> I added the case into `DeclTest.cpp` because i think it can cover the root cause for both testcases.
> 
> How about remove the somewhat repeated one in `ASTImporterTest.cpp`?
Now that you have written it, we can leave both tests i think.
Afaik one test serialization so they complement each other


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158145