rsmith added inline comments.
================
Comment at: clang/include/clang/AST/Decl.h:903
/// declared.
- unsigned ScopeDepthOrObjCQuals : NumScopeDepthOrObjCQualsBits;
+ unsigned ScopeDepthOrObjCQuals : IQ::NumScopeDepthOrObjCQualsBits;
----------------
These bitfields are assumed to fit into an `unsigned`, and this can no longer
be verified locally (changes to the `.td` file can break this assumption).
Please add a `static_assert(sizeof(ParmVarDeclBitfields) <= sizeof(unsigned),
"...")` next to the `union` below (and likewise for `NonParmVarDeclBitfields`).
================
Comment at: clang/include/clang/AST/Decl.h:903
/// declared.
- unsigned ScopeDepthOrObjCQuals : NumScopeDepthOrObjCQualsBits;
+ unsigned ScopeDepthOrObjCQuals : IQ::NumScopeDepthOrObjCQualsBits;
----------------
rsmith wrote:
> These bitfields are assumed to fit into an `unsigned`, and this can no longer
> be verified locally (changes to the `.td` file can break this assumption).
> Please add a `static_assert(sizeof(ParmVarDeclBitfields) <= sizeof(unsigned),
> "...")` next to the `union` below (and likewise for
> `NonParmVarDeclBitfields`).
I think `IQ` is too terse. We don't need a really short name for this; how
about `ImpLimits::` or similar?
================
Comment at: clang/include/clang/Basic/ImplementationQuantities.td:107-111
+ bit isBitLimit = 0;
+ bit isValueLimit = 0;
+ bit isCompilerFlagLimit = 0;
+ bit isDescriptionLimit = 0;
+ bit hasRemark = 0;
----------------
Switching between upper- and lower-case names for these fields seems
surprising. Please pick a consistent capitalization scheme.
================
Comment at: clang/lib/CodeGen/CGRecordLayout.h:72
/// The total size of the bit-field, in bits.
- unsigned Size : 15;
+ unsigned Size : IQ::BitFieldBits;
----------------
These three bitfields no longer obviously fit in 32 bits. Please keep the
hard-coded 15 here and add an adjacent `static_assert(IQ::BitFieldBits <= 15,
"...")` instead, or use some similar strategy so that the bit-field allocation
and layout remains locally obvious.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72053/new/
https://reviews.llvm.org/D72053
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
