[PATCH] D71142: [Sema] Validate large bitfields

2020-04-07 Thread Mark de Wever via Phabricator via cfe-commits
Mordante planned changes to this revision. Mordante added a comment. In D71142#1967409 , @aaron.ballman wrote: > In D71142#1967315 , @xbolva00 wrote: > > > Any comments? > > > > @rsmith @aaron.ballman > > > There

[PATCH] D71142: [Sema] Validate large bitfields

2020-04-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D71142#1967315 , @xbolva00 wrote: > Any comments? > > @rsmith @aaron.ballman There are outstanding review comments, so I'm waiting for the patch author to address those, unless I've missed a question somewhere?

[PATCH] D71142: [Sema] Validate large bitfields

2020-04-07 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Any comments? @rsmith @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71142/new/ https://reviews.llvm.org/D71142 ___ cfe-commits mailing list

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-08 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked an inline comment as done. Mordante added a comment. In D71142#1774270 , @xbolva00 wrote: > Maybe a bit offtopic, but it would be good to diagnose this case too: > > enum A {a, b, c = 8}; > > struct T { > enum A field : 2; >

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-08 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Maybe a bit offtopic, but it would be good to diagnose this case too: enum A {a, b, c = 8}; struct T { enum A field : 2; }; GCC catches this bug. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71142/new/

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-08 Thread 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

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D71142#1773933 , @Mordante wrote: > I'll have a look whether I can find a sane maximum width for a bit-field. The limits according to C are the size of the field's declared type (C17 6.7.2.1p4), but it seems that C++

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-07 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked 3 inline comments as done. Mordante added a comment. I'll have a look whether I can find a sane maximum width for a bit-field. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71142/new/ https://reviews.llvm.org/D71142

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. It seems fine and reasonable to reject this as a violation of an implementation limit. Can we actually support the full range 2^64 bits range, or would it be wiser to pick a smaller limit? (There's at least no point in allowing bit-widths that would mean the size of the

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. 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

[PATCH] D71142: [Sema] Validate large bitfields

2019-12-06 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision. Mordante added reviewers: rsmith, majnemer, aaron.ballman. Mordante added a project: clang. Note: I'm not entirely happy with the text of the diagnostics and I'm open to suggestions. Fixes PR23505: Large bitfield: Assertion `getActiveBits() <= 64 && "Too many