[PATCH] D71142: [Sema] Validate large bitfields

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

Further testing revealed the Codegen already has decided the limit.
`clang/lib/CodeGen/CGRecordLayout.h:64`:

  struct CGBitFieldInfo {
    /// The offset within a contiguous run of bitfields that are represented as
    /// a single "field" within the LLVM struct type. This offset is in bits.
    unsigned Offset : 16;
  
    /// The total size of the bit-field, in bits.
    unsigned Size : 15;

So I'll look at using that limit. I also want to look at improving the diagnostics a bit more.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5186-5189
+def warn_bitfield_width_exceeds_maximum_width: Error<
+  "width of bit-field %0 doesn't fit in a 64 bit unsigned integer">;
+def warn_anon_bitfield_width_exceeds_maximum_width : Error<
+  "width of anonymous bit-field doesn't fit in a 64 bit unsigned integer">;
----------------
aaron.ballman wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > I feel like this situation should be an error rather than a warning -- what could the code possibly have meant?
> > The name of the diagnostic and the kind of diagnostic should agree. Currently we have `warn_` vs `Error<`.
> Ooof, good catch! These diagnostics should be renamed to start with `err_` instead.
Hmm odd I'm sure I left a comment here yesterday, but it seems I didn't commit it properly.
It is a copy paste bug and I forgot to change the `warn_` prefix to `err_`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71142