Skip to content

Commit 6cb7f22

Browse files
plotfiplotfi
committed
[NFC][ClangImporter] Alter importDecl API to return an optional.
Inspired by swiftlang#36747. I thought I'd go a little further. I have found there is quite a lot of usage of dyn_cast_or_null in place of checking for nullptr or using an optional. I think using an llvm::Optional<T> is a lot cleaner.
1 parent 96231e2 commit 6cb7f22

File tree

7 files changed

+306
-210
lines changed

7 files changed

+306
-210
lines changed

‎include/swift/ClangImporter/ClangImporter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ class ClangImporter final : public ClangModuleLoader {
434434

435435
/// If we already imported a given decl, return the corresponding Swift decl.
436436
/// Otherwise, return nullptr.
437-
Decl *importDeclCached(const clang::NamedDecl *ClangDecl);
437+
Optional<Decl *> importDeclCached(const clang::NamedDecl *ClangDecl);
438438

439439
// Returns true if it is expected that the macro is ignored.
440440
bool shouldIgnoreMacro(StringRef Name, const clang::MacroInfo *Macro);

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 75 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2871,8 +2871,8 @@ void ClangImporter::lookupBridgingHeaderDecls(
28712871
for (auto *ClangD : Impl.BridgeHeaderTopLevelDecls) {
28722872
if (filter(ClangD)) {
28732873
if (auto *ND = dyn_cast<clang::NamedDecl>(ClangD)) {
2874-
if (Decl *imported = Impl.importDeclReal(ND, Impl.CurrentVersion))
2875-
receiver(imported);
2874+
if (auto imported = Impl.importDeclReal(ND, Impl.CurrentVersion))
2875+
receiver(imported.getValue());
28762876
}
28772877
}
28782878
}
@@ -2937,8 +2937,8 @@ bool ClangImporter::lookupDeclsFromHeader(StringRef Filename,
29372937
continue;
29382938
if (filter(ClangD)) {
29392939
if (auto *ND = dyn_cast<clang::NamedDecl>(ClangD)) {
2940-
if (Decl *imported = Impl.importDeclReal(ND, Impl.CurrentVersion))
2941-
receiver(imported);
2940+
if (auto imported = Impl.importDeclReal(ND, Impl.CurrentVersion))
2941+
receiver(imported.getValue());
29422942
}
29432943
}
29442944
}
@@ -3035,13 +3035,18 @@ void ClangImporter::lookupTypeDecl(
30353035
!isa<clang::ObjCCompatibleAliasDecl>(clangDecl)) {
30363036
continue;
30373037
}
3038-
Decl *imported = Impl.importDecl(clangDecl, Impl.CurrentVersion);
3038+
const Optional<Decl *> imported =
3039+
Impl.importDecl(clangDecl, Impl.CurrentVersion);
3040+
if (!imported.hasValue())
3041+
continue;
30393042

30403043
// Namespaces are imported as extensions for enums.
3041-
if (auto ext = dyn_cast_or_null<ExtensionDecl>(imported)) {
3042-
imported = ext->getExtendedNominal();
3044+
Decl *importedExtendedNominal = imported.getValue();
3045+
if (auto ext = dyn_cast_or_null<ExtensionDecl>(imported.getValue())) {
3046+
importedExtendedNominal = ext->getExtendedNominal();
30433047
}
3044-
if (auto *importedType = dyn_cast_or_null<TypeDecl>(imported)) {
3048+
if (auto *importedType =
3049+
dyn_cast_or_null<TypeDecl>(importedExtendedNominal)) {
30453050
foundViaClang = true;
30463051
receiver(importedType);
30473052
}
@@ -3147,10 +3152,15 @@ void ClangModuleUnit::getTopLevelDecls(SmallVectorImpl<Decl*> &results) const {
31473152

31483153
// Add the extensions produced by importing categories.
31493154
for (auto category : lookupTable->categories()) {
3150-
if (auto extension = cast_or_null<ExtensionDecl>(
3151-
owner.importDecl(category, owner.CurrentVersion,
3152-
/*UseCanonical*/false))) {
3153-
results.push_back(extension);
3155+
const Optional<Decl *> extension =
3156+
owner.importDecl(category,
3157+
owner.CurrentVersion, /*UseCanonical*/false);
3158+
if (!extension.hasValue())
3159+
continue;
3160+
3161+
if (auto extensionValue =
3162+
cast_or_null<ExtensionDecl>(extension.getValue())) {
3163+
results.push_back(extensionValue);
31543164
}
31553165
}
31563166

@@ -3170,32 +3180,34 @@ void ClangModuleUnit::getTopLevelDecls(SmallVectorImpl<Decl*> &results) const {
31703180
llvm::SmallPtrSet<ExtensionDecl *, 8> knownExtensions;
31713181
for (auto entry : lookupTable->allGlobalsAsMembers()) {
31723182
auto decl = entry.get<clang::NamedDecl *>();
3173-
Decl *importedDecl = owner.importDecl(decl, owner.CurrentVersion);
3183+
const Optional<Decl *> importedDecl =
3184+
owner.importDecl(decl, owner.CurrentVersion);
31743185
if (!importedDecl) continue;
31753186

31763187
// Find the enclosing extension, if there is one.
3177-
ExtensionDecl *ext = findEnclosingExtension(importedDecl);
3188+
ExtensionDecl *ext = findEnclosingExtension(importedDecl.getValue());
31783189
if (ext && knownExtensions.insert(ext).second)
31793190
results.push_back(ext);
31803191

31813192
// If this is a compatibility typealias, the canonical type declaration
31823193
// may exist in another extension.
3183-
auto alias = dyn_cast<TypeAliasDecl>(importedDecl);
3194+
auto alias = dyn_cast<TypeAliasDecl>(importedDecl.getValue());
31843195
if (!alias || !alias->isCompatibilityAlias()) continue;
31853196

31863197
auto aliasedTy = alias->getUnderlyingType();
31873198
ext = nullptr;
3188-
importedDecl = nullptr;
31893199

31903200
// Note: We can't use getAnyGeneric() here because `aliasedTy`
31913201
// might be typealias.
3202+
Decl *aliasTypeDecl = nullptr;
31923203
if (auto Ty = dyn_cast<TypeAliasType>(aliasedTy.getPointer()))
3193-
importedDecl = Ty->getDecl();
3204+
aliasTypeDecl = Ty->getDecl();
31943205
else if (auto Ty = dyn_cast<AnyGenericType>(aliasedTy.getPointer()))
3195-
importedDecl = Ty->getDecl();
3196-
if (!importedDecl) continue;
3206+
aliasTypeDecl = Ty->getDecl();
3207+
else
3208+
continue;
31973209

3198-
ext = findEnclosingExtension(importedDecl);
3210+
ext = findEnclosingExtension(aliasTypeDecl);
31993211
if (ext && knownExtensions.insert(ext).second)
32003212
results.push_back(ext);
32013213
}
@@ -3275,7 +3287,7 @@ void ClangModuleUnit::lookupValue(DeclName name, NLKind lookupKind,
32753287
if (auto lookupTable = owner.findLookupTable(clangModule)) {
32763288
// Search it.
32773289
owner.lookupValue(*lookupTable, name, *consumer);
3278-
if (getASTContext().LangOpts.EnableExperimentalClangImporterDiagnostics) {
3290+
if (getASTContext().LangOpts.EnableExperimentalClangImporterDiagnostics) {
32793291
if (results.empty()) {
32803292
owner.diagnoseValue(*lookupTable, name);
32813293
}
@@ -3359,23 +3371,26 @@ ClangModuleUnit::lookupNestedType(Identifier name,
33593371
if (!newName.getDeclName().isSimpleName(name))
33603372
return true;
33613373

3362-
auto decl = dyn_cast_or_null<TypeDecl>(
3363-
owner.importDeclReal(clangTypeDecl, nameVersion));
3364-
if (!decl)
3374+
const Optional<Decl *> decl =
3375+
owner.importDeclReal(clangTypeDecl, nameVersion);
3376+
if (!decl.hasValue())
3377+
return false;
3378+
auto declValue = dyn_cast_or_null<TypeDecl>(decl.getValue());
3379+
if (!declValue)
33653380
return false;
33663381

33673382
if (!originalDecl)
3368-
originalDecl = decl;
3369-
else if (originalDecl == decl)
3383+
originalDecl = declValue;
3384+
else if (originalDecl == declValue)
33703385
return true;
33713386

3372-
auto *importedContext = decl->getDeclContext()->getSelfNominalTypeDecl();
3387+
auto *importedContext = declValue->getDeclContext()->getSelfNominalTypeDecl();
33733388
if (importedContext != baseType)
33743389
return true;
33753390

3376-
assert(decl->getName() == name &&
3391+
assert(declValue->getName() == name &&
33773392
"importFullName behaved differently from importDecl");
3378-
results.push_back(decl);
3393+
results.push_back(declValue);
33793394
anyMatching = true;
33803395
return true;
33813396
});
@@ -3458,8 +3473,9 @@ void ClangImporter::loadObjCMethods(
34583473
(void)Impl.importDecl(objcMethod->findPropertyDecl(true),
34593474
Impl.CurrentVersion);
34603475

3461-
method = dyn_cast_or_null<AbstractFunctionDecl>(
3462-
Impl.importDecl(objcMethod, Impl.CurrentVersion));
3476+
if (const Optional<Decl *> methodOpt =
3477+
Impl.importDecl(objcMethod, Impl.CurrentVersion))
3478+
method = dyn_cast_or_null<AbstractFunctionDecl>(methodOpt.getValue());
34633479
}
34643480

34653481
// If we didn't find anything, we're done.
@@ -3541,14 +3557,15 @@ void ClangModuleUnit::lookupObjCMethods(
35413557
if (objcMethod->isPropertyAccessor())
35423558
(void)owner.importDecl(objcMethod->findPropertyDecl(true),
35433559
owner.CurrentVersion);
3544-
Decl *imported = owner.importDecl(objcMethod, owner.CurrentVersion);
3560+
Optional<Decl *> imported =
3561+
owner.importDecl(objcMethod, owner.CurrentVersion);
35453562
if (!imported) continue;
35463563

3547-
if (auto func = dyn_cast<AbstractFunctionDecl>(imported))
3564+
if (auto func = dyn_cast<AbstractFunctionDecl>(imported.getValue()))
35483565
results.push_back(func);
35493566

35503567
// If there is an alternate declaration, also look at it.
3551-
for (auto alternate : owner.getAlternateDecls(imported)) {
3568+
for (auto alternate : owner.getAlternateDecls(imported.getValue())) {
35523569
if (auto func = dyn_cast<AbstractFunctionDecl>(alternate))
35533570
results.push_back(func);
35543571
}
@@ -3630,7 +3647,8 @@ std::string ClangImporter::getClangModuleHash() const {
36303647
return Impl.Invocation->getModuleHash(Impl.Instance->getDiagnostics());
36313648
}
36323649

3633-
Decl *ClangImporter::importDeclCached(const clang::NamedDecl *ClangDecl) {
3650+
Optional<Decl *>
3651+
ClangImporter::importDeclCached(const clang::NamedDecl *ClangDecl) {
36343652
return Impl.importDeclCached(ClangDecl, Impl.CurrentVersion);
36353653
}
36363654

@@ -4005,8 +4023,11 @@ bool ClangImporter::Implementation::lookupValue(SwiftLookupTable &table,
40054023
if (name.isOperator()) {
40064024
for (auto entry : table.lookupMemberOperators(name.getBaseName())) {
40074025
if (isVisibleClangEntry(entry)) {
4008-
if (auto decl = dyn_cast_or_null<ValueDecl>(
4009-
importDeclReal(entry->getMostRecentDecl(), CurrentVersion))) {
4026+
const Optional<Decl *> declOpt =
4027+
importDeclReal(entry->getMostRecentDecl(), CurrentVersion);
4028+
if (!declOpt.hasValue())
4029+
continue;
4030+
if (auto decl = dyn_cast_or_null<ValueDecl>(declOpt.getValue())) {
40104031
consumer.foundDecl(decl, DeclVisibilityKind::VisibleAtTopLevel);
40114032
declFound = true;
40124033
}
@@ -4022,14 +4043,14 @@ bool ClangImporter::Implementation::lookupValue(SwiftLookupTable &table,
40224043
// If it's a Clang declaration, try to import it.
40234044
if (auto clangDecl = entry.dyn_cast<clang::NamedDecl *>()) {
40244045
bool isNamespace = isa<clang::NamespaceDecl>(clangDecl);
4025-
Decl *realDecl =
4046+
auto realDecl =
40264047
importDeclReal(clangDecl->getMostRecentDecl(), CurrentVersion,
40274048
/*useCanonicalDecl*/ !isNamespace);
40284049

4029-
if (!realDecl)
4050+
if (!realDecl.hasValue() || !realDecl.getValue() ||
4051+
!isa<ValueDecl>(realDecl.getValue()))
40304052
continue;
4031-
decl = cast<ValueDecl>(realDecl);
4032-
if (!decl) continue;
4053+
decl = cast<ValueDecl>(realDecl.getValue());
40334054
} else if (!name.isSpecial()) {
40344055
// Try to import a macro.
40354056
if (auto modMacro = entry.dyn_cast<clang::ModuleMacro *>())
@@ -4096,9 +4117,12 @@ bool ClangImporter::Implementation::lookupValue(SwiftLookupTable &table,
40964117
return;
40974118

40984119
// Then try to import the decl under the alternate name.
4120+
const Optional<Decl *> alternateNamedDeclOpt =
4121+
importDeclReal(recentClangDecl, nameVersion);
4122+
if (!alternateNamedDeclOpt.hasValue())
4123+
return;
40994124
auto alternateNamedDecl =
4100-
cast_or_null<ValueDecl>(importDeclReal(recentClangDecl,
4101-
nameVersion));
4125+
cast_or_null<ValueDecl>(alternateNamedDeclOpt.getValue());
41024126
if (!alternateNamedDecl || alternateNamedDecl == decl)
41034127
return;
41044128
assert(alternateNamedDecl->getName().matchesRef(name) &&
@@ -4144,8 +4168,10 @@ void ClangImporter::Implementation::lookupObjCMembers(
41444168
[&](ImportedName importedName,
41454169
ImportNameVersion nameVersion) -> bool {
41464170
// Import the declaration.
4147-
auto decl =
4148-
cast_or_null<ValueDecl>(importDeclReal(clangDecl, nameVersion));
4171+
const Optional<Decl *> declOpt = importDeclReal(clangDecl, nameVersion);
4172+
if (!declOpt.hasValue())
4173+
return false;
4174+
auto decl = cast_or_null<ValueDecl>(declOpt.getValue());
41494175
if (!decl)
41504176
return false;
41514177

@@ -4716,8 +4742,10 @@ ClangImporter::instantiateCXXClassTemplate(
47164742
assert(isa<clang::RecordType>(CanonType) &&
47174743
"type of non-dependent specialization is not a RecordType");
47184744

4719-
return dyn_cast_or_null<StructDecl>(
4720-
Impl.importDecl(ctsd, Impl.CurrentVersion));
4745+
Optional<Decl *> structDecl = Impl.importDecl(ctsd, Impl.CurrentVersion);
4746+
if (!structDecl.hasValue())
4747+
return nullptr;
4748+
return dyn_cast_or_null<StructDecl>(structDecl.getValue());
47214749
}
47224750

47234751
bool ClangImporter::isCXXMethodMutating(const clang::CXXMethodDecl *method) {

‎lib/ClangImporter/DWARFImporter.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,15 +156,18 @@ void ClangImporter::Implementation::lookupValueDWARF(
156156
auto *namedDecl = dyn_cast<clang::NamedDecl>(clangDecl);
157157
if (!namedDecl)
158158
continue;
159-
auto *swiftDecl = cast_or_null<ValueDecl>(
160-
importDeclReal(namedDecl->getMostRecentDecl(), CurrentVersion));
159+
const Optional<Decl *> swiftDecl =
160+
importDeclReal(namedDecl->getMostRecentDecl(), CurrentVersion);
161161
if (!swiftDecl)
162162
continue;
163+
auto *swiftDeclValue = cast_or_null<ValueDecl>(swiftDecl.getValue());
164+
if (!swiftDeclValue)
165+
continue;
163166

164-
if (swiftDecl->getName().matchesRef(name) &&
165-
swiftDecl->getDeclContext()->isModuleScopeContext()) {
166-
forceLoadAllMembers(dyn_cast<IterableDeclContext>(swiftDecl));
167-
results.push_back(swiftDecl);
167+
if (swiftDeclValue->getName().matchesRef(name) &&
168+
swiftDeclValue->getDeclContext()->isModuleScopeContext()) {
169+
forceLoadAllMembers(dyn_cast<IterableDeclContext>(swiftDeclValue));
170+
results.push_back(swiftDeclValue);
168171
}
169172
}
170173
}
@@ -185,10 +188,13 @@ void ClangImporter::Implementation::lookupTypeDeclDWARF(
185188
continue;
186189
}
187190
auto *namedDecl = cast<clang::NamedDecl>(clangDecl);
188-
Decl *importedDecl = cast_or_null<ValueDecl>(
189-
importDeclReal(namedDecl->getMostRecentDecl(), CurrentVersion));
191+
const Optional<Decl *> importedDecl =
192+
importDeclReal(namedDecl->getMostRecentDecl(), CurrentVersion);
193+
if (!importedDecl)
194+
continue;
190195

191-
if (auto *importedType = dyn_cast_or_null<TypeDecl>(importedDecl)) {
196+
Decl *importedDeclValue = cast_or_null<ValueDecl>(importedDecl.getValue());
197+
if (auto *importedType = dyn_cast_or_null<TypeDecl>(importedDeclValue)) {
192198
forceLoadAllMembers(dyn_cast<IterableDeclContext>(importedType));
193199
receiver(importedType);
194200
}

0 commit comments

Comments
 (0)