[PATCH] D126642: [Clang] NFCI: Repurpose HasExtParameterInfos for HasExtraBitfields

Sander de Smalen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 31 08:29:57 PDT 2022


sdesmalen added a comment.

In D126642#3547284 <https://reviews.llvm.org/D126642#3547284>, @erichkeane wrote:

> I'm not sure I'm grokking hte difference between the ExtraBitfields and ExtParamInfos here.

The reason I repurposed the bit was because I previously tried adding a bit to `unsigned ExtInfo : 13;` but that fails

  static_assert(sizeof(*this) <= 8 + sizeof(ExtQualsTypeCommonBase), "changing bitfields changed sizeof(Type)!");

so perhaps incorrectly I assumed I couldn't add any new bits to FunctionType and thought I'd repurpose this one bit, because it's only ever used for FunctionProtoType (never for FunctionNoProtoType).

But I now realised I can just add an extra bit, so that we can leave this bit as-is. What do you think?



================
Comment at: clang/include/clang/AST/Type.h:4103
   bool hasExtraBitfields() const {
-    return hasExtraBitfields(getExceptionSpecType());
+    assert((getExceptionSpecType() != EST_Dynamic ||
+            FunctionTypeBits.HasExtraBitfields) &&
----------------
erichkeane wrote:
> Why is asking if we DO have extra bitfields an assert here?  I would think this is a valid question...
> 
> Why would 'dynamic' and extra-bitfields be exclusive here?
This assert is merely confirming that HasExtraBitfields **must** be true if the ExceptionSpecType is `EST_Dynamic`, because that was the old behaviour (and I wanted to avoid introducing failures if some code still assumed that hasExtraBitfields == true, but for some reason HasExtraBitfields had not yet been set to `true`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126642



More information about the cfe-commits mailing list