hubert.reinterpretcast added inline comments.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1884
+ if (const BuiltinType *BTy =
+ Context.getBaseElementType(CTy->getElementType())
+ ->getAs<BuiltinType>())
----------------
I don't think there's a reason to use `getBaseElementType` (which is used to
handle arrays) on the element type of a `ComplexType`.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1885
+ Context.getBaseElementType(CTy->getElementType())
+ ->getAs<BuiltinType>())
+ if (BTy->getKind() == BuiltinType::Double ||
----------------
I believe `castAs` should be expected to succeed here.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1888
+ BTy->getKind() == BuiltinType::LongDouble) {
+ PreferredAlign = CharUnits::fromQuantity(8);
+ }
----------------
I believe an assertion that `PreferredAlign` was 4 would be appropriate.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1893
+ ->getAs<BuiltinType>()) {
+ if (BTy->getKind() == BuiltinType::Double ||
+ BTy->getKind() == BuiltinType::LongDouble) {
----------------
Use a lambda instead of duplicating the code.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1898
+ } else if (const RecordType *RT = D->getType()
+ ->getBaseElementTypeUnsafe()
+ ->getAs<RecordType>()) {
----------------
Is there a reason to use `getBaseElementTypeUnsafe` for this case and
`Context.getBaseElementType` for the other ones? Also, it would make sense to
factor out the array-type considerations once at the top of the if-else chain
instead of doing so in each alternative.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1900
+ ->getAs<RecordType>()) {
+ if (const RecordDecl *RD = RT->getDecl()) {
+ const ASTRecordLayout &FieldRecord = Context.getASTRecordLayout(RD);
----------------
I'd be a bit concerned if this failed. Can we assert that we get a non-null
pointer back?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79719/new/
https://reviews.llvm.org/D79719
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
