gbencze added inline comments.
================
Comment at: clang/lib/AST/ASTContext.cpp:2582-2584
+ int64_t BitfieldSize = Field->getBitWidthValue(Context);
+ if (BitfieldSize > FieldSizeInBits)
+ return llvm::None;
----------------
steakhal wrote:
> Why do you return `None` here?
These bits were just extracted from `structHasUniqueObjectRepresentations`, the
underlying logic was not changed here. (originally in line 2615)
I believe this handles the case "If the width of a bit-field is larger than the
width of the bit-field's type (or, in case of an enumeration type, of its
underlying type), the extra bits are padding bits" from [class.bit]
================
Comment at: clang/lib/AST/ASTContext.cpp:2595-2598
+template <typename RangeT>
+static llvm::Optional<int64_t> structSubobjectsHaveUniqueObjectRepresentations(
+ const RangeT &Subobjects, int64_t CurOffsetInBits, const ASTContext
&Context,
+ const clang::ASTRecordLayout &Layout) {
----------------
steakhal wrote:
> Why is this a template?
> Can't you just take the `field_range`?
>
> By the same token, `structSubobjectsHaveUniqueObjectRepresentations` returns
> an `optional int`. I'm confused.
This function is also called with (non virtual) base class subobjects, not just
fields.
A `None` return value indicates that the subobjects do not have unique object
representations, otherwise the size of the subobjects is returned. This allows
us to detect for example padding between a base class and the first field.
================
Comment at: clang/test/SemaCXX/has_unique_object_reps_no_unique_addr.cpp:42
+} // namespace TailPaddingReuse
+static_assert(__has_unique_object_representations(TailPaddingReuse::B));
----------------
steakhal wrote:
> Why is this outside of the namespace declaration?
Tbh I'm not sure. I can move it in the namespace if you think it'd be better
there.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89649/new/
https://reviews.llvm.org/D89649
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
