Skip to content

Commit 7a0d474

Browse files
committed
[LLDB] Remove Swift AST-based name translation from MTC
The name translation only affects selectors and not classes, so it already isn't correct in all cases. The translation is implemented on top of SwiftASTContext, which is very expensive to start up and on top of that may fail if the compiler flags aren't all available. Right now the cost-benefit trade-off for this feature doesn't work out. We should look into reimplementing it on top of TypeSystemSwiftTypeRef::GetSwiftName() which uses the APINotes and ClangImporter's name translation engine to do the job. rdar://162496659 (cherry picked from commit 1b1fed2)
1 parent 2bd78f9 commit 7a0d474

File tree

2 files changed

+11
-81
lines changed

2 files changed

+11
-81
lines changed

lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp

Lines changed: 5 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -85,84 +85,11 @@ bool InstrumentationRuntimeMainThreadChecker::CheckIfRuntimeIsValid(
8585
static std::string TranslateObjCNameToSwiftName(std::string className,
8686
std::string selector,
8787
StackFrameSP swiftFrame) {
88-
if (className.empty() || selector.empty())
89-
return "";
90-
ModuleSP swiftModule = swiftFrame->GetFrameCodeAddress().GetModule();
91-
if (!swiftModule)
92-
return "";
93-
94-
auto type_system_or_err = swiftModule->GetTypeSystemForLanguage(lldb::eLanguageTypeSwift);
95-
if (!type_system_or_err) {
96-
llvm::consumeError(type_system_or_err.takeError());
97-
return "";
98-
}
99-
100-
auto *ts = llvm::dyn_cast_or_null<TypeSystemSwift>(type_system_or_err->get());
101-
if (!ts)
102-
return "";
103-
const SymbolContext *sc = nullptr;
104-
if (swiftFrame)
105-
sc = &swiftFrame->GetSymbolContext(eSymbolContextFunction);
106-
if (!sc)
107-
return "";
108-
auto ctx = ts->GetSwiftASTContext(*sc);
109-
if (!ctx)
110-
return "";
111-
swift::ClangImporter *imp = ctx->GetClangImporter();
112-
if (!imp)
113-
return "";
114-
115-
size_t numArguments = llvm::StringRef(selector).count(':');
116-
llvm::SmallVector<llvm::StringRef, 4> parts;
117-
llvm::StringRef(selector).split(parts, ":", /*MaxSplit*/ -1,
118-
/*KeepEmpty*/ false);
119-
120-
llvm::SmallVector<swift::Identifier, 2> selectorIdentifiers;
121-
for (size_t i = 0; i < parts.size(); i++) {
122-
selectorIdentifiers.push_back(ctx->GetIdentifier(parts[i]));
123-
}
124-
125-
class MyConsumer : public swift::VisibleDeclConsumer {
126-
public:
127-
swift::ObjCSelector selectorToLookup;
128-
swift::DeclName result;
129-
130-
MyConsumer(swift::ObjCSelector selector) : selectorToLookup(selector) {}
131-
132-
void foundDecl(swift::ValueDecl *VD,
133-
swift::DeclVisibilityKind Reason,
134-
swift::DynamicLookupInfo) override{
135-
if (result)
136-
return; // Take the first result.
137-
swift::ClassDecl *cls = llvm::dyn_cast<swift::ClassDecl>(VD);
138-
if (!cls)
139-
return;
140-
auto funcs = cls->lookupDirect(selectorToLookup, true);
141-
if (funcs.size() == 0)
142-
return;
143-
144-
// If the decl is actually an accessor, use the property name instead.
145-
swift::AbstractFunctionDecl *decl = funcs.front();
146-
if (auto accessor = llvm::dyn_cast<swift::AccessorDecl>(decl)) {
147-
result = accessor->getStorage()->getName();
148-
return;
149-
}
150-
151-
result = decl->getName();
152-
}
153-
};
154-
155-
ThreadSafeASTContext ast_ctx = ctx->GetASTContext();
156-
MyConsumer consumer(swift::ObjCSelector(**ast_ctx, numArguments,
157-
selectorIdentifiers));
158-
// FIXME(mracek): Switch to a new API that translates the Clang class name
159-
// to Swift class name, once this API exists. Now we assume they are the same.
160-
imp->lookupValue(ctx->GetIdentifier(className), consumer);
161-
162-
if (!consumer.result)
163-
return "";
164-
llvm::SmallString<32> scratchSpace;
165-
return className + "." + consumer.result.getString(scratchSpace).str();
88+
// FIXME: This used to be implemented in terms of Swift AST
89+
// operations. We should reimplement this on top of
90+
// TypeSystemSwiftTypeRef::GetSwiftName() which uses the APINotes
91+
// and ClangImporter's name translation engine to do the job.
92+
return className + "." + selector;
16693
}
16794
#endif // LLDB_ENABLE_SWIFT
16895

lldb/test/API/functionalities/mtc/swift/TestMTCSwift.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ def mtc_tests(self):
3939

4040
self.expect("thread info",
4141
substrs=['stop reason = ' + view +
42-
'.removeFromSuperview() must be used from main thread only'])
42+
# FIXME: should be: .removeFromSuperview()
43+
'.removeFromSuperview must be used from main thread only'])
4344

4445
self.expect(
4546
"thread info -s",
@@ -50,7 +51,9 @@ def mtc_tests(self):
5051
json_line = '\n'.join(output_lines[2:])
5152
data = json.loads(json_line)
5253
self.assertEqual(data["instrumentation_class"], "MainThreadChecker")
53-
self.assertEqual(data["api_name"], view + ".removeFromSuperview()")
54+
# FIXME: .removeFromSuperview()
55+
self.assertEqual(data["api_name"], view + ".removeFromSuperview")
5456
self.assertEqual(data["class_name"], view)
5557
self.assertEqual(data["selector"], "removeFromSuperview")
56-
self.assertEqual(data["description"], view + ".removeFromSuperview() must be used from main thread only")
58+
# FIXME: .removeFromSuperview()
59+
self.assertEqual(data["description"], view + ".removeFromSuperview must be used from main thread only")

0 commit comments

Comments
 (0)