Skip to content

Commit 7c1e394

Browse files
CodaFiCodaFi
committed
[NFC] Refactor Override Fixit Emission
Refactor diagnostic emission so it lazily emits notes and fixits. This is a necessary evil in a world where an arbitrary request can also emit diagnostics, especially circularity diagnostics. In the future, we should assert in the evaluator that there is no active diagnostic to catch the rest of these.
1 parent 3574c51 commit 7c1e394

File tree

4 files changed

+84
-50
lines changed

4 files changed

+84
-50
lines changed

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1799,9 +1799,15 @@ void swift::diagnoseUnownedImmediateDeallocation(TypeChecker &TC,
17991799
}
18001800
}
18011801

1802-
bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
1803-
ValueDecl *decl,
1804-
const ValueDecl *base) {
1802+
namespace {
1803+
enum NoteKind_t {
1804+
FixItReplace,
1805+
FixItInsert,
1806+
};
1807+
1808+
static bool fixItOverrideDeclarationTypesImpl(
1809+
ValueDecl *decl, const ValueDecl *base,
1810+
SmallVectorImpl<std::tuple<NoteKind_t, SourceRange, std::string>> &notes) {
18051811
// For now, just rewrite cases where the base uses a value type and the
18061812
// override uses a reference type, and the value type is bridged to the
18071813
// reference type. This is a way to migrate code that makes use of types
@@ -1864,7 +1870,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
18641870
options.SynthesizeSugarOnTypes = true;
18651871

18661872
newOverrideTy->print(baseTypeStr, options);
1867-
diag.fixItReplace(typeRange, baseTypeStr.str());
1873+
notes.emplace_back(FixItReplace, typeRange, baseTypeStr.str().str());
18681874
return true;
18691875
};
18701876

@@ -1884,7 +1890,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
18841890
overrideFnTy->getExtInfo().isNoEscape() &&
18851891
// The overridden function type should be escaping.
18861892
!baseFnTy->getExtInfo().isNoEscape()) {
1887-
diag.fixItInsert(typeRange.Start, "@escaping ");
1893+
notes.emplace_back(FixItInsert, typeRange, "@escaping ");
18881894
return true;
18891895
}
18901896
return false;
@@ -1922,7 +1928,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
19221928
for_each(*fn->getParameters(),
19231929
*baseFn->getParameters(),
19241930
[&](ParamDecl *param, const ParamDecl *baseParam) {
1925-
fixedAny |= fixItOverrideDeclarationTypes(diag, param, baseParam);
1931+
fixedAny |= fixItOverrideDeclarationTypesImpl(param, baseParam, notes);
19261932
});
19271933
}
19281934
if (auto *method = dyn_cast<FuncDecl>(decl)) {
@@ -1948,7 +1954,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
19481954
for_each(*subscript->getIndices(),
19491955
*baseSubscript->getIndices(),
19501956
[&](ParamDecl *param, const ParamDecl *baseParam) {
1951-
fixedAny |= fixItOverrideDeclarationTypes(diag, param, baseParam);
1957+
fixedAny |= fixItOverrideDeclarationTypesImpl(param, baseParam, notes);
19521958
});
19531959
}
19541960

@@ -1964,6 +1970,26 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
19641970

19651971
llvm_unreachable("unknown overridable member");
19661972
}
1973+
};
1974+
1975+
bool swift::computeFixitsForOverridenDeclaration(
1976+
ValueDecl *decl, const ValueDecl *base,
1977+
llvm::function_ref<Optional<InFlightDiagnostic>(bool)> diag) {
1978+
SmallVector<std::tuple<NoteKind_t, SourceRange, std::string>, 4> Notes;
1979+
bool hasNotes = ::fixItOverrideDeclarationTypesImpl(decl, base, Notes);
1980+
1981+
Optional<InFlightDiagnostic> diagnostic = diag(hasNotes);
1982+
if (!diagnostic) return hasNotes;
1983+
1984+
for (const auto &note : Notes) {
1985+
if (std::get<0>(note) == FixItReplace) {
1986+
diagnostic->fixItReplace(std::get<1>(note), std::get<2>(note));
1987+
} else {
1988+
diagnostic->fixItInsert(std::get<1>(note).Start, std::get<2>(note));
1989+
}
1990+
}
1991+
return hasNotes;
1992+
}
19671993

19681994
//===----------------------------------------------------------------------===//
19691995
// Per func/init diagnostics

‎lib/Sema/MiscDiagnostics.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,15 @@ void diagnoseUnownedImmediateDeallocation(TypeChecker &TC,
9090
/// \p base...but only if we're highly confident that we know what the user
9191
/// should have written.
9292
///
93+
/// The \p diag closure allows the caller to control the diagnostic that is
94+
/// emitted. It is passed true if the diagnostic will be emitted with fixits
95+
/// attached, and false otherwise. If None is returned, no diagnostics are
96+
/// emitted. Else the fixits are attached to the returned diagnostic.
97+
///
9398
/// \returns true iff any fix-its were attached to \p diag.
94-
bool fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
95-
ValueDecl *decl,
96-
const ValueDecl *base);
99+
bool computeFixitsForOverridenDeclaration(
100+
ValueDecl *decl, const ValueDecl *base,
101+
llvm::function_ref<Optional<InFlightDiagnostic>(bool)> diag);
97102

98103
/// Emit fix-its to enclose trailing closure in argument parens.
99104
void fixItEncloseTrailingClosure(TypeChecker &TC,

‎lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -422,44 +422,40 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base);
422422
static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
423423
auto &ctx = decl->getASTContext();
424424
auto &diags = ctx.Diags;
425-
DiagnosticTransaction tentativeDiags(diags);
426425

427-
{
428-
Type baseTy = base->getInterfaceType();
429-
if (baseTy->hasError())
430-
return false;
431-
432-
Optional<InFlightDiagnostic> activeDiag;
433-
if (auto *baseInit = dyn_cast<ConstructorDecl>(base)) {
434-
// Special-case initializers, whose "type" isn't useful besides the
435-
// input arguments.
436-
auto *fnType = baseTy->getAs<AnyFunctionType>();
437-
baseTy = fnType->getResult();
438-
Type argTy = FunctionType::composeInput(ctx,
439-
baseTy->getAs<AnyFunctionType>()
440-
->getParams(),
441-
false);
442-
auto diagKind = diag::override_type_mismatch_with_fixits_init;
443-
unsigned numArgs = baseInit->getParameters()->size();
444-
activeDiag.emplace(diags.diagnose(decl, diagKind,
445-
/*plural*/std::min(numArgs, 2U),
446-
argTy));
447-
} else {
448-
if (isa<AbstractFunctionDecl>(base))
449-
baseTy = baseTy->getAs<AnyFunctionType>()->getResult();
450-
451-
activeDiag.emplace(
452-
diags.diagnose(decl,
453-
diag::override_type_mismatch_with_fixits,
454-
base->getDescriptiveKind(), baseTy));
455-
}
426+
Type baseTy = base->getInterfaceType();
427+
if (baseTy->hasError())
428+
return false;
456429

457-
if (fixItOverrideDeclarationTypes(*activeDiag, decl, base))
458-
return true;
430+
if (auto *baseInit = dyn_cast<ConstructorDecl>(base)) {
431+
// Special-case initializers, whose "type" isn't useful besides the
432+
// input arguments.
433+
auto *fnType = baseTy->getAs<AnyFunctionType>();
434+
baseTy = fnType->getResult();
435+
Type argTy = FunctionType::composeInput(
436+
ctx, baseTy->getAs<AnyFunctionType>()->getParams(), false);
437+
auto diagKind = diag::override_type_mismatch_with_fixits_init;
438+
unsigned numArgs = baseInit->getParameters()->size();
439+
return computeFixitsForOverridenDeclaration(
440+
decl, base, [&](bool HasNotes) -> Optional<InFlightDiagnostic> {
441+
if (!HasNotes)
442+
return None;
443+
return diags.diagnose(decl, diagKind,
444+
/*plural*/ std::min(numArgs, 2U), argTy);
445+
});
446+
} else {
447+
if (isa<AbstractFunctionDecl>(base))
448+
baseTy = baseTy->getAs<AnyFunctionType>()->getResult();
449+
450+
return computeFixitsForOverridenDeclaration(
451+
decl, base, [&](bool HasNotes) -> Optional<InFlightDiagnostic> {
452+
if (!HasNotes)
453+
return None;
454+
return diags.diagnose(decl, diag::override_type_mismatch_with_fixits,
455+
base->getDescriptiveKind(), baseTy);
456+
});
459457
}
460458

461-
// There weren't any fixes we knew how to make. Drop this diagnostic.
462-
tentativeDiags.abort();
463459
return false;
464460
}
465461

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,12 +2069,19 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
20692069
break;
20702070

20712071
case MatchKind::TypeConflict: {
2072-
auto diag = diags.diagnose(match.Witness,
2073-
diag::protocol_witness_type_conflict,
2074-
getTypeForDisplay(module, match.Witness),
2075-
withAssocTypes);
2076-
if (!isa<TypeDecl>(req))
2077-
fixItOverrideDeclarationTypes(diag, match.Witness, req);
2072+
if (!isa<TypeDecl>(req)) {
2073+
computeFixitsForOverridenDeclaration(match.Witness, req, [&](bool){
2074+
return diags.diagnose(match.Witness,
2075+
diag::protocol_witness_type_conflict,
2076+
getTypeForDisplay(module, match.Witness),
2077+
withAssocTypes);
2078+
});
2079+
} else {
2080+
diags.diagnose(match.Witness,
2081+
diag::protocol_witness_type_conflict,
2082+
getTypeForDisplay(module, match.Witness),
2083+
withAssocTypes);
2084+
}
20782085
break;
20792086
}
20802087

0 commit comments

Comments
 (0)