steakhal added a comment.
It looks great.
================
Comment at: clang/lib/AST/ASTContext.cpp:2582-2584
+ int64_t BitfieldSize = Field->getBitWidthValue(Context);
+ if (BitfieldSize > FieldSizeInBits)
+ return llvm::None;
----------------
Why do you return `None` here?
================
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) {
----------------
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.
================
Comment at: clang/test/SemaCXX/has_unique_object_reps_no_unique_addr.cpp:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify
-std=c++2a %s
+// expected-no-diagnostics
----------------
gbencze wrote:
> Just to be sure: is the specifying the triple here enough to ensure that this
> always uses the Itanium ABI? I believe MSVC currently ignores the
> `no_unique_address` attribute. Or do I need to add some more flags?
> Alternatively, the static_asserts could be modified to check `sizeof(T) >
> [expected size with Itanium ABI] || __has_unique_object_representations(T)`
I think it's better to specify the triple exactly
================
Comment at: clang/test/SemaCXX/has_unique_object_reps_no_unique_addr.cpp:42
+} // namespace TailPaddingReuse
+static_assert(__has_unique_object_representations(TailPaddingReuse::B));
----------------
Why is this outside of the namespace declaration?
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
