This is an archive of the discontinued LLVM Phabricator instance.

Differential D38774

[CodeGen] Add support for IncompleteArrayType in Obj-C ivars.
ClosedPublic

Authored by vsapsai on Oct 10 2017, 5:33 PM.

Details

Reviewers
rjmccall
theraven
Commits
rG636ed47428c1: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.
rC316723: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.
rL316723: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.
Summary

Fixes an assertion failure when ivar is a struct containing incomplete
array. Also completes support for direct flexible array members.

rdar://problem/21054495

Diff Detail

Event Timeline

vsapsai created this revision.Oct 10 2017, 5:33 PM
vsapsai added a comment.Oct 10 2017, 5:38 PM

This patch depends on sema change https://reviews.llvm.org/D38773

vsapsai added a comment.Oct 10 2017, 6:06 PM

To save reviewers some time. Incomplete array type is not the same as for zero-sized array, it is c"^c\00" while for zero-sized array it is c"[0c]\00". Don't know if it matters, I haven't found any difference in behaviour. Though maybe I wasn't looking in the right place.

dexonsmith added a parent revision: D38114: [Sema] Emit an error for using tags with flexible array member as ObjC class ivar..Oct 10 2017, 8:20 PM
dexonsmith edited parent revisions, added: D38773: [Sema] Add support for flexible array members in Obj-C.; removed: D38114: [Sema] Emit an error for using tags with flexible array member as ObjC class ivar..
rjmccall added inline comments.Oct 22 2017, 8:34 PM
clang/lib/CodeGen/CGObjCMac.cpp
5095

You can't just use isa<> here; there can be typedefs of incomplete array type.

vsapsai added inline comments.Oct 23 2017, 3:34 PM
clang/lib/CodeGen/CGObjCMac.cpp
5095

Thanks for catching it.

vsapsai updated this revision to Diff 119951.Oct 23 2017, 3:38 PM
  • Address rjmccall review comment about isa<>.

Decided to keep in test only cases with typedefs because test coverage is the
same and there is less similar code.

vsapsai updated this revision to Diff 119952.Oct 23 2017, 3:42 PM
  • Resubmit my last change without files from underlying branch.
Harbormaster completed remote builds in B11405: Diff 119952.Oct 23 2017, 3:42 PM
rjmccall added inline comments.Oct 23 2017, 3:44 PM
clang/lib/CodeGen/CGObjCMac.cpp
5095

Oh, and C allows you to have incomplete arrays of array type, e.g. int x[][4];

vsapsai added inline comments.Oct 25 2017, 4:23 PM
clang/lib/CodeGen/CGObjCMac.cpp
5095

Right, thanks. I've checked only that int x[][]; isn't accepted but forgot about mixing different arrays.

vsapsai updated this revision to Diff 120337.Oct 25 2017, 4:23 PM
  • Address rjmccall review comment about nested arrays.
Harbormaster completed remote builds in B11505: Diff 120337.Oct 25 2017, 4:23 PM
rjmccall accepted this revision.Oct 26 2017, 1:08 PM

Thanks, LGTM.

This revision is now accepted and ready to land.Oct 26 2017, 1:08 PM
Closed by commit rL316723: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars. (authored by vsapsai). · Explain WhyOct 26 2017, 5:56 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
clang/
lib/
CodeGen/
5 lines
test/
CodeGenObjC/
28 lines

Diff 119952

clang/lib/CodeGen/CGObjCMac.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 5,083 Lines • ▼ Show 20 Linesvoid IvarLayoutBuilder::visitField(const FieldDecl *field,
// Drill down into arrays. // Drill down into arrays.
uint64_t numElts = 1; uint64_t numElts = 1;
while (auto arrayType = CGM.getContext().getAsConstantArrayType(fieldType)) { while (auto arrayType = CGM.getContext().getAsConstantArrayType(fieldType)) {
numElts *= arrayType->getSize().getZExtValue(); numElts *= arrayType->getSize().getZExtValue();
fieldType = arrayType->getElementType(); fieldType = arrayType->getElementType();
} }
if (auto arrayType = CGM.getContext().getAsIncompleteArrayType(fieldType)) {
numElts = 0;
fieldType = arrayType->getElementType();
}
rjmccallUnsubmitted
Not Done
ReplyInline Actions

You can't just use isa<> here; there can be typedefs of incomplete array type.

rjmccall: You can't just use isa<> here; there can be typedefs of incomplete array type.
vsapsaiAuthorUnsubmitted
Not Done
ReplyInline Actions

Thanks for catching it.

vsapsai: Thanks for catching it.
rjmccallUnsubmitted
Not Done
ReplyInline Actions

Oh, and C allows you to have incomplete arrays of array type, e.g. int x[][4];

rjmccall: Oh, and C allows you to have incomplete arrays of array type, e.g. int x[][4];
vsapsaiAuthorUnsubmitted
Not Done
ReplyInline Actions

Right, thanks. I've checked only that int x[][]; isn't accepted but forgot about mixing different arrays.

vsapsai: Right, thanks. I've checked only that `int x[][];` isn't accepted but forgot about mixing…
assert(!fieldType->isArrayType() && "ivar of non-constant array type?"); assert(!fieldType->isArrayType() && "ivar of non-constant array type?");
// If we ended up with a zero-sized array, we've done what we can do within // If we ended up with a zero-sized array, we've done what we can do within
// the limits of this layout encoding. // the limits of this layout encoding.
if (numElts == 0) return; if (numElts == 0) return;
// Recurse if the base element type is a record type. // Recurse if the base element type is a record type.
if (auto recType = fieldType->getAs<RecordType>()) { if (auto recType = fieldType->getAs<RecordType>()) {
▲ Show 20 Lines • Show All 2,506 Lines • Show Last 20 Lines

clang/test/CodeGenObjC/ivar-layout-flexible-array.m

  • This file was added.
// RUN: %clang_cc1 -triple x86_64-apple-darwin -Wno-objc-root-class -fobjc-arc -emit-llvm -o - %s | FileCheck %s
// rdar://problem/21054495
typedef char FlexibleArray[];
@interface FlexibleArrayMember {
FlexibleArray flexible_array;
}
@end
@implementation FlexibleArrayMember
@end
// CHECK: @OBJC_METH_VAR_NAME_{{.*}} = private unnamed_addr constant {{.*}} c"flexible_array\00"
// CHECK-NEXT: @OBJC_METH_VAR_TYPE_{{.*}} = private unnamed_addr constant {{.*}} c"^c\00"
struct Packet {
int size;
FlexibleArray data;
};
@interface VariableSizeIvar {
struct Packet flexible_struct;
}
@end
@implementation VariableSizeIvar
@end
// CHECK: @OBJC_METH_VAR_NAME_{{.*}} = private unnamed_addr constant {{.*}} c"flexible_struct\00"
// CHECK-NEXT: @OBJC_METH_VAR_TYPE_{{.*}} = private unnamed_addr constant {{.*}} c"{Packet=\22size\22i\22data\22[0c]}\00"