From 1b1fed239062bda4551f5dea2a3544d2648b5f53 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Wed, 15 Oct 2025 17:40:56 -0700 Subject: [PATCH] [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 --- ...nstrumentationRuntimeMainThreadChecker.cpp | 83 ++----------------- .../functionalities/mtc/swift/TestMTCSwift.py | 9 +- 2 files changed, 11 insertions(+), 81 deletions(-) diff --git a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp index b9592f562b4c9..1076fb69bbba2 100644 --- a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp +++ b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp @@ -85,84 +85,11 @@ bool InstrumentationRuntimeMainThreadChecker::CheckIfRuntimeIsValid( static std::string TranslateObjCNameToSwiftName(std::string className, std::string selector, StackFrameSP swiftFrame) { - if (className.empty() || selector.empty()) - return ""; - ModuleSP swiftModule = swiftFrame->GetFrameCodeAddress().GetModule(); - if (!swiftModule) - return ""; - - auto type_system_or_err = swiftModule->GetTypeSystemForLanguage(lldb::eLanguageTypeSwift); - if (!type_system_or_err) { - llvm::consumeError(type_system_or_err.takeError()); - return ""; - } - - auto *ts = llvm::dyn_cast_or_null(type_system_or_err->get()); - if (!ts) - return ""; - const SymbolContext *sc = nullptr; - if (swiftFrame) - sc = &swiftFrame->GetSymbolContext(eSymbolContextFunction); - if (!sc) - return ""; - auto ctx = ts->GetSwiftASTContext(*sc); - if (!ctx) - return ""; - swift::ClangImporter *imp = ctx->GetClangImporter(); - if (!imp) - return ""; - - size_t numArguments = llvm::StringRef(selector).count(':'); - llvm::SmallVector parts; - llvm::StringRef(selector).split(parts, ":", /*MaxSplit*/ -1, - /*KeepEmpty*/ false); - - llvm::SmallVector selectorIdentifiers; - for (size_t i = 0; i < parts.size(); i++) { - selectorIdentifiers.push_back(ctx->GetIdentifier(parts[i])); - } - - class MyConsumer : public swift::VisibleDeclConsumer { - public: - swift::ObjCSelector selectorToLookup; - swift::DeclName result; - - MyConsumer(swift::ObjCSelector selector) : selectorToLookup(selector) {} - - void foundDecl(swift::ValueDecl *VD, - swift::DeclVisibilityKind Reason, - swift::DynamicLookupInfo) override{ - if (result) - return; // Take the first result. - swift::ClassDecl *cls = llvm::dyn_cast(VD); - if (!cls) - return; - auto funcs = cls->lookupDirect(selectorToLookup, true); - if (funcs.size() == 0) - return; - - // If the decl is actually an accessor, use the property name instead. - swift::AbstractFunctionDecl *decl = funcs.front(); - if (auto accessor = llvm::dyn_cast(decl)) { - result = accessor->getStorage()->getName(); - return; - } - - result = decl->getName(); - } - }; - - ThreadSafeASTContext ast_ctx = ctx->GetASTContext(); - MyConsumer consumer(swift::ObjCSelector(**ast_ctx, numArguments, - selectorIdentifiers)); - // FIXME(mracek): Switch to a new API that translates the Clang class name - // to Swift class name, once this API exists. Now we assume they are the same. - imp->lookupValue(ctx->GetIdentifier(className), consumer); - - if (!consumer.result) - return ""; - llvm::SmallString<32> scratchSpace; - return className + "." + consumer.result.getString(scratchSpace).str(); + // FIXME: This used to be implemented in terms of Swift AST + // operations. We should reimplement this on top of + // TypeSystemSwiftTypeRef::GetSwiftName() which uses the APINotes + // and ClangImporter's name translation engine to do the job. + return className + "." + selector; } #endif // LLDB_ENABLE_SWIFT diff --git a/lldb/test/API/functionalities/mtc/swift/TestMTCSwift.py b/lldb/test/API/functionalities/mtc/swift/TestMTCSwift.py index 772c1f700fbd1..dd92af8db8ef3 100644 --- a/lldb/test/API/functionalities/mtc/swift/TestMTCSwift.py +++ b/lldb/test/API/functionalities/mtc/swift/TestMTCSwift.py @@ -39,7 +39,8 @@ def mtc_tests(self): self.expect("thread info", substrs=['stop reason = ' + view + - '.removeFromSuperview() must be used from main thread only']) + # FIXME: should be: .removeFromSuperview() + '.removeFromSuperview must be used from main thread only']) self.expect( "thread info -s", @@ -50,7 +51,9 @@ def mtc_tests(self): json_line = '\n'.join(output_lines[2:]) data = json.loads(json_line) self.assertEqual(data["instrumentation_class"], "MainThreadChecker") - self.assertEqual(data["api_name"], view + ".removeFromSuperview()") + # FIXME: .removeFromSuperview() + self.assertEqual(data["api_name"], view + ".removeFromSuperview") self.assertEqual(data["class_name"], view) self.assertEqual(data["selector"], "removeFromSuperview") - self.assertEqual(data["description"], view + ".removeFromSuperview() must be used from main thread only") + # FIXME: .removeFromSuperview() + self.assertEqual(data["description"], view + ".removeFromSuperview must be used from main thread only")