Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve findinstances handling of Swift subclasses #219

Merged
merged 6 commits into from
Jan 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Chisel/Chisel.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
7ABD17981DF7F998006118F8 /* ChiselTests */,
7ABD178C1DF7F998006118F8 /* Products */,
);
indentWidth = 2;
sourceTree = "<group>";
};
7ABD178C1DF7F998006118F8 /* Products */ = {
Expand Down
52 changes: 39 additions & 13 deletions Chisel/Chisel/CHLObjcInstanceCommands.mm
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ static bool objectIsMatch(NSPredicate *predicate, id obj, const std::unordered_s

// Function reimplementation of +[NSObject isSubclassOf:] to avoid the objc runtime side
// effects that can happen when calling methods, like realizing classes, +initialize, etc.
static bool isSubclassOf(Class base, Class target)
static bool isSubclassOfClass(Class self, Class target)
{
for (auto cls = base; cls != Nil; cls = class_getSuperclass(cls)) {
for (auto cls = self; cls != Nil; cls = class_getSuperclass(cls)) {
if (cls == target) {
return true;
}
Expand All @@ -115,9 +115,9 @@ static bool isSubclassOf(Class base, Class target)

// Function reimplementation of +[NSObject conformsToProtocol:] to avoid the objc runtime side
// effects that can happen when calling methods, like realizing classes, +initialize, etc.
static bool conformsToProtocol(Class base, Protocol *protocol)
static bool conformsToProtocol(Class self, Protocol *protocol)
{
for (auto cls = base; cls != Nil; cls = class_getSuperclass(cls)) {
for (auto cls = self; cls != Nil; cls = class_getSuperclass(cls)) {
if (class_conformsToProtocol(cls, protocol)) {
return true;
}
Expand All @@ -129,7 +129,12 @@ void PrintInstances(const char *type, const char *pred)
{
NSPredicate *predicate = nil;
if (pred != nullptr && *pred != '\0') {
predicate = [NSPredicate predicateWithFormat:@(pred)];
@try {
predicate = [NSPredicate predicateWithFormat:@(pred)];
} @catch (NSException *e) {
printf("Error: Invalid predicate; %s\n", [e reason].UTF8String);
return;
}
}

const std::unordered_set<Class> objcClasses = CHLObjcClassSet();
Expand All @@ -144,17 +149,38 @@ void PrintInstances(const char *type, const char *pred)
}
}

bool exactClass = false;
if (type[0] == '*') {
exactClass = true;
++type;
Class cls = objc_getClass(type);
if (cls != nullptr) {
matchClasses.insert(cls);
}

// Helper lambda that only exists so that it can be called more than once, as in the
// rare case where `type` corresponds to more than one Swift class.
auto addMatch = [&](Class baseClass) {
if (exactClass) {
matchClasses.insert(baseClass);
} else {
for (auto cls : objcClasses) {
if (isSubclassOfClass(cls, baseClass)) {
matchClasses.insert(cls);
}
}
}
} else if (Class kind = objc_getClass(type)) {
// This could be optimized for type == "NSObject", but it won't be a typical search.
};

Class baseClass = objc_getClass(type);
if (baseClass != Nil) {
addMatch(baseClass);
} else {
// The given class name hasn't been found, this could be a Swift class which has
// a module name prefix. Loop over all classes to look for matching class names.
for (auto cls : objcClasses) {
if (isSubclassOf(cls, kind)) {
matchClasses.insert(cls);
// SwiftModule.ClassName
// ^- dot + 1
auto dot = strchr(class_getName(cls), '.');
if (dot && strcmp(type, dot + 1) == 0) {
addMatch(cls);
}
}
}
Expand All @@ -167,7 +193,7 @@ void PrintInstances(const char *type, const char *pred)

NSSet *keyPaths = CHLVariableKeyPaths(predicate);

std::vector<id, zone_allocator<id>> instances = CHLScanObjcInstances(matchClasses);
auto instances = CHLScanObjcInstances(matchClasses);
unsigned int matches = 0;

for (id obj : instances) {
Expand Down
4 changes: 2 additions & 2 deletions Chisel/Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PREFIX ?= /usr/local/lib

export INSTALL_NAME = ""
ifneq ($(LD_DYLIB_INSTALL_NAME), "")
export INSTALL_NAME =
ifneq ($(LD_DYLIB_INSTALL_NAME),)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keith does this change look legit? The original here was still adding LD_DYLIB_INSTALL_NAME=... to xcodebuild command, but this change appears to fix that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea seems fine, shouldn't have had the quotes in the first place I think, my bad.

INSTALL_NAME = "LD_DYLIB_INSTALL_NAME=$(LD_DYLIB_INSTALL_NAME)"
endif

Expand Down