Skip to content

[cxx-interop] Remove some duplicated lookups #79324

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 27, 2025
Merged

[cxx-interop] Remove some duplicated lookups #79324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 16 additions & 12 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2478,8 +2478,9 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
// well, and may do unnecessary work.
ClangModuleUnit *wrapperUnit = getWrapperForModule(clangModule, importLoc);
ModuleDecl *result = wrapperUnit->getParentModule();
if (!ModuleWrappers[clangModule].getInt()) {
ModuleWrappers[clangModule].setInt(true);
auto &moduleWrapper = ModuleWrappers[clangModule];
if (!moduleWrapper.getInt()) {
moduleWrapper.setInt(true);
(void) namelookup::getAllImports(result);
}

Expand Down Expand Up @@ -2889,11 +2890,11 @@ void ClangImporter::Implementation::addImportDiagnostic(
ImportDiagnosticTarget target, Diagnostic &&diag,
clang::SourceLocation loc) {
ImportDiagnostic importDiag = ImportDiagnostic(target, diag, loc);
if (SwiftContext.LangOpts.DisableExperimentalClangImporterDiagnostics ||
CollectedDiagnostics.count(importDiag))
if (SwiftContext.LangOpts.DisableExperimentalClangImporterDiagnostics)
return;
auto [_, inserted] = CollectedDiagnostics.insert(importDiag);
if (!inserted)
return;

CollectedDiagnostics.insert(importDiag);
ImportDiagnostics[target].push_back(importDiag);
}

Expand Down Expand Up @@ -6846,8 +6847,9 @@ void ClangImporter::Implementation::dumpSwiftLookupTables() {
// Print out the lookup tables for the various modules.
for (auto moduleName : moduleNames) {
llvm::errs() << "<<" << moduleName << " lookup table>>\n";
LookupTables[moduleName]->deserializeAll();
LookupTables[moduleName]->dump(llvm::errs());
auto &lookupTable = LookupTables[moduleName];
lookupTable->deserializeAll();
lookupTable->dump(llvm::errs());
}

llvm::errs() << "<<Bridging header lookup table>>\n";
Expand Down Expand Up @@ -7410,8 +7412,10 @@ ClangImporter::getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
if (!newFn)
return ConcreteDeclRef(decl);

if (Impl.specializedFunctionTemplates.count(newFn))
return ConcreteDeclRef(Impl.specializedFunctionTemplates[newFn]);
auto [fnIt, inserted] =
Impl.specializedFunctionTemplates.try_emplace(newFn, nullptr);
if (!inserted)
return ConcreteDeclRef(fnIt->second);

auto newDecl = cast_or_null<ValueDecl>(
decl->getASTContext().getClangModuleLoader()->importDeclDirectly(
Expand All @@ -7434,7 +7438,7 @@ ClangImporter::getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
}
}

Impl.specializedFunctionTemplates[newFn] = newDecl;
fnIt->getSecond() = newDecl;
return ConcreteDeclRef(newDecl);
}

Expand Down Expand Up @@ -8006,7 +8010,7 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator,
mod->lookupValue(desc.ctx.getIdentifier(swiftName), NLKind::UnqualifiedLookup,
results);
if (results.size() == 1) {
if (dyn_cast<ClassDecl>(results[0]))
if (isa<ClassDecl>(results[0]))
return results[0];
}
return nullptr;
Expand Down
7 changes: 4 additions & 3 deletions lib/ClangImporter/DWARFImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,15 @@ ModuleDecl *ClangImporter::Implementation::loadModuleDWARF(

// FIXME: Implement submodule support!
Identifier name = path[0].Item;
auto it = DWARFModuleUnits.find(name);
if (it != DWARFModuleUnits.end())
auto [it, inserted] = DWARFModuleUnits.try_emplace(name, nullptr);
if (!inserted)
return it->second->getParentModule();

auto itCopy = it; // Capturing structured bindings is a C++20 feature.
auto *M = ModuleDecl::create(name, SwiftContext,
[&](ModuleDecl *M, auto addFile) {
auto *wrapperUnit = new (SwiftContext) DWARFModuleUnit(*M, *this);
DWARFModuleUnits.insert({name, wrapperUnit});
itCopy->second = wrapperUnit;
addFile(wrapperUnit);
});
M->setIsNonSwiftModule();
Expand Down
37 changes: 18 additions & 19 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/TinyPtrVector.h"
#include "llvm/Support/Path.h"

#include <algorithm>
Expand Down Expand Up @@ -2578,11 +2579,11 @@ namespace {
Impl.addAlternateDecl(subscriptImpl, subscript);
}

if (Impl.cxxDereferenceOperators.find(result) !=
Impl.cxxDereferenceOperators.end()) {
auto getterAndSetterIt = Impl.cxxDereferenceOperators.find(result);
if (getterAndSetterIt != Impl.cxxDereferenceOperators.end()) {
// If this type has a dereference operator, synthesize a computed
// property called `pointee` for it.
auto getterAndSetter = Impl.cxxDereferenceOperators[result];
auto getterAndSetter = getterAndSetterIt->second;

VarDecl *pointeeProperty =
synthesizer.makeDereferencedPointeeProperty(
Expand Down Expand Up @@ -5076,10 +5077,9 @@ namespace {

// Check whether there's some special method to import.
if (!forceClassMethod) {
if (dc == Impl.importDeclContextOf(decl, decl->getDeclContext()) &&
!Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}])
Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}]
= result;
if (dc == Impl.importDeclContextOf(decl, decl->getDeclContext()))
Impl.ImportedDecls.try_emplace(
{decl->getCanonicalDecl(), getVersion()}, result);

if (importedName.isSubscriptAccessor()) {
// If this was a subscript accessor, try to create a
Expand Down Expand Up @@ -7696,8 +7696,8 @@ SwiftDeclConverter::importSubscript(Decl *decl,

// Note that we've created this subscript.
Impl.Subscripts[{getter, setter}] = subscript;
if (setter && !Impl.Subscripts[{getter, nullptr}])
Impl.Subscripts[{getter, nullptr}] = subscript;
if (setter)
Impl.Subscripts.try_emplace({getter, nullptr}, subscript);

// Make the getter/setter methods unavailable.
if (!getter->isUnavailable())
Expand Down Expand Up @@ -8413,11 +8413,12 @@ SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
StringRef attributeText,
bool cached
) {
::TinyPtrVector<SourceFile *> *sourceFiles = nullptr;
if (cached) {
auto &sourceFiles = ClangSwiftAttrSourceFiles[attributeText];
sourceFiles = &ClangSwiftAttrSourceFiles[attributeText];

// Check whether we've already created a source file.
for (auto sourceFile : sourceFiles) {
for (auto sourceFile : *sourceFiles) {
if (sourceFile->getParentModule() == &module)
return *sourceFile;
}
Expand All @@ -8443,10 +8444,8 @@ SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
auto sourceFile = new (SwiftContext)
SourceFile(module, SourceFileKind::Library, bufferID);

if (cached) {
auto &sourceFiles = ClangSwiftAttrSourceFiles[attributeText];
sourceFiles.push_back(sourceFile);
}
if (cached)
sourceFiles->push_back(sourceFile);

return *sourceFile;
}
Expand Down Expand Up @@ -9839,9 +9838,9 @@ ClangImporter::Implementation::importDeclContextOf(
// Clang submodule of the declaration.
const clang::Module *declSubmodule = *getClangSubmoduleForDecl(decl);
auto extensionKey = std::make_pair(nominal, declSubmodule);
auto knownExtension = extensionPoints.find(extensionKey);
if (knownExtension != extensionPoints.end())
return knownExtension->second;
auto [it, inserted] = extensionPoints.try_emplace(extensionKey, nullptr);
if (!inserted)
return it->getSecond();

// Create a new extension for this nominal type/Clang submodule pair.
auto ext = ExtensionDecl::create(SwiftContext, SourceLoc(), nullptr, {},
Expand All @@ -9854,7 +9853,7 @@ ClangImporter::Implementation::importDeclContextOf(
// Record this extension so we can find it later. We do this early because
// once we've set the member loader, we don't know when the compiler will use
// it and end up back in this method.
extensionPoints[extensionKey] = ext;
it->getSecond() = ext;
ext->setMemberLoader(this, reinterpret_cast<uintptr_t>(declSubmodule));

if (auto protoDecl = ext->getExtendedProtocolDecl()) {
Expand Down
7 changes: 4 additions & 3 deletions lib/ClangImporter/ImportMacro.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,10 +779,11 @@ ValueDecl *ClangImporter::Implementation::importMacro(Identifier name,
PrettyStackTraceStringAction stackRAII{"importing macro", name.str()};

// Look for macros imported with the same name.
auto known = ImportedMacros.find(name);
if (known == ImportedMacros.end()) {
auto [known, inserted] = ImportedMacros.try_emplace(
name, SmallVector<std::pair<const clang::MacroInfo *, ValueDecl *>, 2>{});
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use something like auto here to avoid repeating the type 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.

Unfortunately, I could not avoid naming the type here because the value for try_emplace is taking a template so there is no context to deduce the type of this SmallVector from.

if (inserted) {
// Push in a placeholder to break circularity.
ImportedMacros[name].push_back({macro, nullptr});
known->getSecond().push_back({macro, nullptr});
} else {
// Check whether this macro has already been imported.
for (const auto &entry : known->second) {
Expand Down
6 changes: 3 additions & 3 deletions lib/ClangImporter/ImportType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3574,8 +3574,8 @@ static ModuleDecl *tryLoadModule(ASTContext &C,
llvm::DenseMap<Identifier, ModuleDecl *>
&checkedModules) {
// If we've already done this check, return the cached result.
auto known = checkedModules.find(moduleName);
if (known != checkedModules.end())
auto [known, inserted] = checkedModules.try_emplace(moduleName, nullptr);
if (!inserted)
return known->second;

ModuleDecl *module;
Expand All @@ -3587,7 +3587,7 @@ static ModuleDecl *tryLoadModule(ASTContext &C,
else
module = C.getModuleByIdentifier(moduleName);

checkedModules[moduleName] = module;
known->getSecond() = module;
return module;
}

Expand Down
4 changes: 0 additions & 4 deletions lib/ClangImporter/ImporterImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,10 +801,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
llvm::DenseMap<const Decl *, ArrayRef<ProtocolDecl *>>
ImportedProtocols;

/// The set of declaration context for which we've already ruled out the
/// presence of globals-as-members.
llvm::DenseSet<const IterableDeclContext *> checkedGlobalsAsMembers;

void startedImportingEntity();

public:
Expand Down