Skip to content

Commit a475f4c

Browse files
committed
[Serialization|NFC] Intro diagnoseAndConsumeError
Intro the service `diagnoseAndConsumeError` as the ultimate site to drop deserialization issues we can recover from. It will be used to raise diagnostics on the issues before dropping them silently.
1 parent e0014b4 commit a475f4c

File tree

3 files changed

+40
-31
lines changed

3 files changed

+40
-31
lines changed

lib/Serialization/Deserialization.cpp

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,7 +1458,7 @@ ModuleFile::getSubstitutionMapChecked(serialization::SubstitutionMapID id) {
14581458
for (auto typeID : replacementTypeIDs) {
14591459
auto typeOrError = getTypeChecked(typeID);
14601460
if (!typeOrError) {
1461-
consumeError(typeOrError.takeError());
1461+
diagnoseAndConsumeError(typeOrError.takeError());
14621462
continue;
14631463
}
14641464
replacementTypes.push_back(typeOrError.get());
@@ -1713,7 +1713,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
17131713
if (maybeType.errorIsA<FatalDeserializationError>())
17141714
return maybeType.takeError();
17151715
// FIXME: Don't throw away the inner error's information.
1716-
consumeError(maybeType.takeError());
1716+
diagnoseAndConsumeError(maybeType.takeError());
17171717
return llvm::make_error<XRefError>("couldn't decode type",
17181718
pathTrace, name);
17191719
}
@@ -2083,7 +2083,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
20832083
return maybeType.takeError();
20842084

20852085
// FIXME: Don't throw away the inner error's information.
2086-
consumeError(maybeType.takeError());
2086+
diagnoseAndConsumeError(maybeType.takeError());
20872087
return llvm::make_error<XRefError>("couldn't decode type",
20882088
pathTrace, memberName);
20892089
}
@@ -2922,7 +2922,7 @@ class DeclDeserializer {
29222922

29232923
auto maybeType = MF.getTypeChecked(typeID);
29242924
if (!maybeType) {
2925-
llvm::consumeError(maybeType.takeError());
2925+
MF.diagnoseAndConsumeError(maybeType.takeError());
29262926
continue;
29272927
}
29282928
inheritedTypes.push_back(
@@ -3248,10 +3248,10 @@ class DeclDeserializer {
32483248
return overriddenOrError.takeError();
32493249
} else if (MF.allowCompilerErrors()) {
32503250
// Drop overriding relationship when allowing errors.
3251-
llvm::consumeError(overriddenOrError.takeError());
3251+
MF.diagnoseAndConsumeError(overriddenOrError.takeError());
32523252
overridden = nullptr;
32533253
} else {
3254-
llvm::consumeError(overriddenOrError.takeError());
3254+
MF.diagnoseAndConsumeError(overriddenOrError.takeError());
32553255
if (overriddenAffectsABI || !ctx.LangOpts.EnableDeserializationRecovery) {
32563256
return llvm::make_error<OverrideError>(name, errorFlags,
32573257
numVTableEntries);
@@ -3395,7 +3395,7 @@ class DeclDeserializer {
33953395
if (overridden.errorIsA<FatalDeserializationError>())
33963396
return overridden.takeError();
33973397

3398-
llvm::consumeError(overridden.takeError());
3398+
MF.diagnoseAndConsumeError(overridden.takeError());
33993399

34003400
return llvm::make_error<OverrideError>(
34013401
name, getErrorFlags(), numVTableEntries);
@@ -3507,7 +3507,7 @@ class DeclDeserializer {
35073507

35083508
// FIXME: This is actually wrong. We can't just drop stored properties
35093509
// willy-nilly if the struct is @frozen.
3510-
consumeError(backingDecl.takeError());
3510+
MF.diagnoseAndConsumeError(backingDecl.takeError());
35113511
return var;
35123512
}
35133513

@@ -3724,15 +3724,15 @@ class DeclDeserializer {
37243724
overridden = overriddenOrError.get();
37253725
} else {
37263726
if (overriddenAffectsABI || !ctx.LangOpts.EnableDeserializationRecovery) {
3727-
llvm::consumeError(overriddenOrError.takeError());
3727+
MF.diagnoseAndConsumeError(overriddenOrError.takeError());
37283728
return llvm::make_error<OverrideError>(name, errorFlags,
37293729
numVTableEntries);
37303730
}
37313731
// Pass through deserialization errors.
37323732
if (overriddenOrError.errorIsA<FatalDeserializationError>())
37333733
return overriddenOrError.takeError();
37343734

3735-
llvm::consumeError(overriddenOrError.takeError());
3735+
MF.diagnoseAndConsumeError(overriddenOrError.takeError());
37363736
overridden = nullptr;
37373737
}
37383738

@@ -4054,7 +4054,7 @@ class DeclDeserializer {
40544054
return pattern.takeError();
40554055

40564056
// Silently drop the pattern...
4057-
llvm::consumeError(pattern.takeError());
4057+
MF.diagnoseAndConsumeError(pattern.takeError());
40584058
// ...but continue to read any further patterns we're expecting.
40594059
continue;
40604060
}
@@ -4572,7 +4572,7 @@ class DeclDeserializer {
45724572
// Pass through deserialization errors.
45734573
if (overridden.errorIsA<FatalDeserializationError>())
45744574
return overridden.takeError();
4575-
llvm::consumeError(overridden.takeError());
4575+
MF.diagnoseAndConsumeError(overridden.takeError());
45764576

45774577
DeclDeserializationError::Flags errorFlags;
45784578
return llvm::make_error<OverrideError>(
@@ -5096,7 +5096,7 @@ llvm::Error DeclDeserializer::deserializeCustomAttrs() {
50965096
// is safe to drop when it can't be deserialized.
50975097
// rdar://problem/56599179. When allowing errors we're doing a best
50985098
// effort to create a module, so ignore in that case as well.
5099-
consumeError(deserialized.takeError());
5099+
MF.diagnoseAndConsumeError(deserialized.takeError());
51005100
} else
51015101
return deserialized.takeError();
51025102
} else if (!deserialized.get() && MF.allowCompilerErrors()) {
@@ -6028,7 +6028,7 @@ Expected<Type> DESERIALIZE_TYPE(NAME_ALIAS_TYPE)(
60286028

60296029
// We're going to recover by falling back to the underlying type, so
60306030
// just ignore the error.
6031-
llvm::consumeError(aliasOrError.takeError());
6031+
MF.diagnoseAndConsumeError(aliasOrError.takeError());
60326032
}
60336033

60346034
if (!alias || !alias->getDeclaredInterfaceType()->isEqual(underlyingType)) {
@@ -7355,7 +7355,7 @@ llvm::Error ModuleFile::consumeExpectedError(llvm::Error &&error) {
73557355
if (error.isA<XRefNonLoadedModuleError>() ||
73567356
error.isA<UnsafeDeserializationError>() ||
73577357
error.isA<ModularizationError>()) {
7358-
consumeError(std::move(error));
7358+
diagnoseAndConsumeError(std::move(error));
73597359
return llvm::Error::success();
73607360
}
73617361

@@ -7369,7 +7369,7 @@ llvm::Error ModuleFile::consumeExpectedError(llvm::Error &&error) {
73697369
if (TE->underlyingReasonIsA<XRefNonLoadedModuleError>() ||
73707370
TE->underlyingReasonIsA<UnsafeDeserializationError>() ||
73717371
TE->underlyingReasonIsA<ModularizationError>()) {
7372-
consumeError(std::move(errorInfo));
7372+
diagnoseAndConsumeError(std::move(errorInfo));
73737373
return llvm::Error::success();
73747374
}
73757375

@@ -7379,6 +7379,11 @@ llvm::Error ModuleFile::consumeExpectedError(llvm::Error &&error) {
73797379
return std::move(error);
73807380
}
73817381

7382+
void ModuleFile::diagnoseAndConsumeError(llvm::Error error) const {
7383+
7384+
consumeError(std::move(error));
7385+
}
7386+
73827387
namespace {
73837388
class LazyConformanceLoaderInfo final
73847389
: llvm::TrailingObjects<LazyConformanceLoaderInfo,
@@ -7447,7 +7452,7 @@ ModuleFile::loadAllConformances(const Decl *D, uint64_t contextData,
74477452
// Ignore if allowing errors, it's just doing a best effort to produce
74487453
// *some* module anyway.
74497454
if (allowCompilerErrors())
7450-
consumeError(std::move(unconsumedError));
7455+
diagnoseAndConsumeError(std::move(unconsumedError));
74517456
else
74527457
fatal(std::move(unconsumedError));
74537458
}
@@ -7537,7 +7542,7 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
75377542
if (maybeConformance) {
75387543
reqConformances.push_back(maybeConformance.get());
75397544
} else if (allowCompilerErrors()) {
7540-
consumeError(maybeConformance.takeError());
7545+
diagnoseAndConsumeError(maybeConformance.takeError());
75417546
reqConformances.push_back(ProtocolConformanceRef::forInvalid());
75427547
} else {
75437548
fatal(maybeConformance.takeError());
@@ -7605,7 +7610,7 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
76057610
second = *secondOrError;
76067611
} else if (getContext().LangOpts.EnableDeserializationRecovery) {
76077612
second = ErrorType::get(getContext());
7608-
consumeError(secondOrError.takeError());
7613+
diagnoseAndConsumeError(secondOrError.takeError());
76097614
} else {
76107615
fatal(secondOrError.takeError());
76117616
}
@@ -7615,7 +7620,7 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
76157620
third = cast_or_null<TypeDecl>(*thirdOrError);
76167621
} else if (getContext().LangOpts.EnableDeserializationRecovery) {
76177622
third = nullptr;
7618-
consumeError(thirdOrError.takeError());
7623+
diagnoseAndConsumeError(thirdOrError.takeError());
76197624
} else {
76207625
fatal(thirdOrError.takeError());
76217626
}
@@ -7656,7 +7661,7 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
76567661
if (deserializedReq) {
76577662
req = cast_or_null<ValueDecl>(*deserializedReq);
76587663
} else if (getContext().LangOpts.EnableDeserializationRecovery) {
7659-
consumeError(deserializedReq.takeError());
7664+
diagnoseAndConsumeError(deserializedReq.takeError());
76607665
req = nullptr;
76617666
needToFillInOpaqueValueWitnesses = true;
76627667
} else {
@@ -7673,7 +7678,7 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
76737678
// In that case, we want the conformance to still be available, but
76747679
// we can't make use of the relationship to the underlying decl.
76757680
} else if (getContext().LangOpts.EnableDeserializationRecovery) {
7676-
consumeError(deserializedWitness.takeError());
7681+
diagnoseAndConsumeError(deserializedWitness.takeError());
76777682
isOpaque = true;
76787683
witness = nullptr;
76797684
} else {
@@ -7706,7 +7711,7 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
77067711
if (witnessSubstitutions.errorIsA<XRefNonLoadedModuleError>() ||
77077712
witnessSubstitutions.errorIsA<UnsafeDeserializationError>() ||
77087713
allowCompilerErrors()) {
7709-
consumeError(witnessSubstitutions.takeError());
7714+
diagnoseAndConsumeError(witnessSubstitutions.takeError());
77107715
isOpaque = true;
77117716
}
77127717
else

lib/Serialization/ModuleFile.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ void ModuleFile::lookupValue(DeclName name,
328328
if (!declOrError) {
329329
if (!getContext().LangOpts.EnableDeserializationRecovery)
330330
fatal(declOrError.takeError());
331-
llvm::consumeError(declOrError.takeError());
331+
diagnoseAndConsumeError(declOrError.takeError());
332332
continue;
333333
}
334334
auto VD = cast<ValueDecl>(declOrError.get());
@@ -347,7 +347,7 @@ void ModuleFile::lookupValue(DeclName name,
347347
if (!declOrError) {
348348
if (!getContext().LangOpts.EnableDeserializationRecovery)
349349
fatal(declOrError.takeError());
350-
llvm::consumeError(declOrError.takeError());
350+
diagnoseAndConsumeError(declOrError.takeError());
351351
continue;
352352
}
353353
auto VD = cast<ValueDecl>(declOrError.get());
@@ -589,7 +589,7 @@ void ModuleFile::lookupVisibleDecls(ImportPath::Access accessPath,
589589
if (!declOrError) {
590590
if (!getContext().LangOpts.EnableDeserializationRecovery)
591591
fatal(declOrError.takeError());
592-
llvm::consumeError(declOrError.takeError());
592+
diagnoseAndConsumeError(declOrError.takeError());
593593
return;
594594
}
595595
consumer.foundDecl(cast<ValueDecl>(declOrError.get()),
@@ -638,7 +638,7 @@ void ModuleFile::loadExtensions(NominalTypeDecl *nominal) {
638638
if (!declOrError) {
639639
if (!getContext().LangOpts.EnableDeserializationRecovery)
640640
fatal(declOrError.takeError());
641-
llvm::consumeError(declOrError.takeError());
641+
diagnoseAndConsumeError(declOrError.takeError());
642642
}
643643
}
644644
} else {
@@ -651,7 +651,7 @@ void ModuleFile::loadExtensions(NominalTypeDecl *nominal) {
651651
if (!declOrError) {
652652
if (!getContext().LangOpts.EnableDeserializationRecovery)
653653
fatal(declOrError.takeError());
654-
llvm::consumeError(declOrError.takeError());
654+
diagnoseAndConsumeError(declOrError.takeError());
655655
}
656656
}
657657
}
@@ -708,7 +708,7 @@ void ModuleFile::loadDerivativeFunctionConfigurations(
708708
if (!derivativeGenSigOrError) {
709709
if (!getContext().LangOpts.EnableDeserializationRecovery)
710710
fatal(derivativeGenSigOrError.takeError());
711-
llvm::consumeError(derivativeGenSigOrError.takeError());
711+
diagnoseAndConsumeError(derivativeGenSigOrError.takeError());
712712
}
713713
auto derivativeGenSig = derivativeGenSigOrError.get();
714714
// NOTE(TF-1038): Result indices are currently unsupported in derivative
@@ -882,7 +882,7 @@ void ModuleFile::lookupClassMembers(ImportPath::Access accessPath,
882882
if (!decl) {
883883
if (!getContext().LangOpts.EnableDeserializationRecovery)
884884
fatal(decl.takeError());
885-
llvm::consumeError(decl.takeError());
885+
diagnoseAndConsumeError(decl.takeError());
886886
continue;
887887
}
888888

@@ -905,7 +905,7 @@ void ModuleFile::lookupClassMembers(ImportPath::Access accessPath,
905905
if (!decl) {
906906
if (!getContext().LangOpts.EnableDeserializationRecovery)
907907
fatal(decl.takeError());
908-
llvm::consumeError(decl.takeError());
908+
diagnoseAndConsumeError(decl.takeError());
909909
continue;
910910
}
911911

lib/Serialization/ModuleFile.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,10 @@ class ModuleFile
418418
/// error is returned.
419419
llvm::Error consumeExpectedError(llvm::Error &&error);
420420

421+
/// Report project errors as remarks if desired, otherwise drop the error
422+
/// silently.
423+
void diagnoseAndConsumeError(llvm::Error error) const;
424+
421425
/// Report an unexpected format error that could happen only from a
422426
/// memory-level inconsistency. Please prefer passing an error to
423427
/// `fatal(llvm::Error error)` when possible.

0 commit comments

Comments
 (0)