Skip to content

Commit 8530969

Browse files
authored
Merge pull request #38327 from slavapestov/invalid-generic-function-type-canonicalization
Don't canonicalize invalid GenericFunctionTypes in diagnostics
2 parents 6a80196 + e83fd4a commit 8530969

File tree

2 files changed

+32
-22
lines changed

2 files changed

+32
-22
lines changed

include/swift/AST/Types.h

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -492,13 +492,7 @@ class alignas(1 << TypeAlignInBits) TypeBase {
492492
TypeBase *getWithoutSyntaxSugar();
493493

494494
/// getASTContext - Return the ASTContext that this type belongs to.
495-
ASTContext &getASTContext() {
496-
// If this type is canonical, it has the ASTContext in it.
497-
if (isCanonical())
498-
return *const_cast<ASTContext*>(Context);
499-
// If not, canonicalize it to get the Context.
500-
return *const_cast<ASTContext*>(getCanonicalType()->Context);
501-
}
495+
ASTContext &getASTContext();
502496

503497
/// isEqual - Return true if these two types are equal, ignoring sugar.
504498
///
@@ -5994,6 +5988,22 @@ class PlaceholderType : public TypeBase {
59945988
};
59955989
DEFINE_EMPTY_CAN_TYPE_WRAPPER(PlaceholderType, Type)
59965990

5991+
/// getASTContext - Return the ASTContext that this type belongs to.
5992+
inline ASTContext &TypeBase::getASTContext() {
5993+
// If this type is canonical, it has the ASTContext in it.
5994+
if (isCanonical())
5995+
return *const_cast<ASTContext*>(Context);
5996+
5997+
// getCanonicalType() on GenericFunctionType is very expensive;
5998+
// instead we can more easily fish the ASTContext out of any
5999+
// structural sub-component.
6000+
if (auto *genericFnType = dyn_cast<GenericFunctionType>(this))
6001+
return genericFnType->getGenericParams()[0]->getASTContext();
6002+
6003+
// If not, canonicalize it to get the Context.
6004+
return *const_cast<ASTContext*>(getCanonicalType()->Context);
6005+
}
6006+
59976007
inline bool TypeBase::isTypeVariableOrMember() {
59986008
if (is<TypeVariableType>())
59996009
return true;

lib/AST/DiagnosticEngine.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,19 @@ static bool isInterestingTypealias(Type type) {
502502
return true;
503503
}
504504

505+
/// Walks the type recursivelly desugaring types to display, but skipping
506+
/// `GenericTypeParamType` because we would lose association with its original
507+
/// declaration and end up presenting the parameter in τ_0_0 format on
508+
/// diagnostic.
509+
static Type getAkaTypeForDisplay(Type type) {
510+
return type.transform([](Type visitTy) -> Type {
511+
if (isa<SugarType>(visitTy.getPointer()) &&
512+
!isa<GenericTypeParamType>(visitTy.getPointer()))
513+
return getAkaTypeForDisplay(visitTy->getDesugaredType());
514+
return visitTy;
515+
});
516+
}
517+
505518
/// Decide whether to show the desugared type or not. We filter out some
506519
/// cases to avoid too much noise.
507520
static bool shouldShowAKA(Type type, StringRef typeName) {
@@ -517,7 +530,7 @@ static bool shouldShowAKA(Type type, StringRef typeName) {
517530
// If they are textually the same, don't show them. This can happen when
518531
// they are actually different types, because they exist in different scopes
519532
// (e.g. everyone names their type parameters 'T').
520-
if (typeName == type->getCanonicalType()->getString())
533+
if (typeName == getAkaTypeForDisplay(type).getString())
521534
return false;
522535

523536
return true;
@@ -532,7 +545,7 @@ static bool typeSpellingIsAmbiguous(Type type,
532545
for (auto arg : Args) {
533546
if (arg.getKind() == DiagnosticArgumentKind::Type) {
534547
auto argType = arg.getAsType();
535-
if (argType && !argType->isEqual(type) &&
548+
if (argType && argType->getWithoutParens().getPointer() != type.getPointer() &&
536549
argType->getWithoutParens().getString(PO) == type.getString(PO)) {
537550
return true;
538551
}
@@ -541,19 +554,6 @@ static bool typeSpellingIsAmbiguous(Type type,
541554
return false;
542555
}
543556

544-
/// Walks the type recursivelly desugaring types to display, but skipping
545-
/// `GenericTypeParamType` because we would lose association with its original
546-
/// declaration and end up presenting the parameter in τ_0_0 format on
547-
/// diagnostic.
548-
static Type getAkaTypeForDisplay(Type type) {
549-
return type.transform([](Type visitTy) -> Type {
550-
if (isa<SugarType>(visitTy.getPointer()) &&
551-
!isa<GenericTypeParamType>(visitTy.getPointer()))
552-
return getAkaTypeForDisplay(visitTy->getDesugaredType());
553-
return visitTy;
554-
});
555-
}
556-
557557
/// Determine whether this is the main actor type.
558558
static bool isMainActor(Type type) {
559559
if (auto nominal = type->getAnyNominal()) {

0 commit comments

Comments
 (0)