From bdfa8d9099cd7529e64a314b3fda270acdf182bb Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 21 Sep 2020 18:01:38 -0700 Subject: [PATCH 1/3] [AST] Allow hole types to originate from declarations Associating holes directly with declarations is useful in cases when declarations are invalid e.g. directly `ErrorType` or have error types inside. --- include/swift/AST/Types.h | 14 +++++++------- lib/AST/ASTContext.cpp | 8 ++------ lib/AST/ASTDumper.cpp | 8 +++++--- lib/AST/ASTPrinter.cpp | 11 +++++++++-- lib/Sema/CSSimplify.cpp | 2 +- lib/Sema/TypeCheckCodeCompletion.cpp | 2 +- 6 files changed, 25 insertions(+), 20 deletions(-) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index 1248445c78518..ef18cc0778694 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -5736,19 +5736,19 @@ DEFINE_EMPTY_CAN_TYPE_WRAPPER(TypeVariableType, Type) /// because the expression is ambiguous. This type is only used by the /// constraint solver and transformed into UnresolvedType to be used in AST. class HoleType : public TypeBase { - using OriginatorType = - llvm::PointerUnion; + using Originator = llvm::PointerUnion; - OriginatorType Originator; + Originator O; - HoleType(ASTContext &C, OriginatorType originator, + HoleType(ASTContext &C, Originator originator, RecursiveTypeProperties properties) - : TypeBase(TypeKind::Hole, &C, properties), Originator(originator) {} + : TypeBase(TypeKind::Hole, &C, properties), O(originator) {} public: - static Type get(ASTContext &ctx, OriginatorType originatorType); + static Type get(ASTContext &ctx, Originator originator); - OriginatorType getOriginatorType() const { return Originator; } + Originator getOriginator() const { return O; } static bool classof(const TypeBase *T) { return T->getKind() == TypeKind::Hole; diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 56df3f0c26057..c1a66d8765bcc 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -2400,13 +2400,9 @@ Type ErrorType::get(Type originalType) { return entry = new (mem) ErrorType(ctx, originalType, properties); } -Type HoleType::get(ASTContext &ctx, OriginatorType originator) { +Type HoleType::get(ASTContext &ctx, Originator originator) { assert(originator); - auto properties = reinterpret_cast(originator.getOpaqueValue()) - ->getRecursiveProperties(); - properties |= RecursiveTypeProperties::HasTypeHole; - - auto arena = getArena(properties); + auto arena = getArena(RecursiveTypeProperties::HasTypeHole); return new (ctx, arena) HoleType(ctx, originator, RecursiveTypeProperties::HasTypeHole); } diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index f2fea5f0535d6..07ee77f9310a4 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -3512,12 +3512,14 @@ namespace { void visitHoleType(HoleType *T, StringRef label) { printCommon(label, "hole_type"); - auto originatorTy = T->getOriginatorType(); - if (auto *typeVar = originatorTy.dyn_cast()) { + auto originator = T->getOriginator(); + if (auto *typeVar = originator.dyn_cast()) { printRec("type_variable", typeVar); + } else if (auto *VD = originator.dyn_cast()) { + VD->dumpRef(PrintWithColorRAII(OS, DeclColor).getOS()); } else { printRec("dependent_member_type", - originatorTy.get()); + originator.get()); } PrintWithColorRAII(OS, ParenthesisColor) << ')'; } diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index cd0838d80466e..1a923c47830dd 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3822,8 +3822,15 @@ class TypePrinter : public TypeVisitor { void visitHoleType(HoleType *T) { if (Options.PrintTypesForDebugging) { Printer << "<getOriginatorType(); - visit(Type(reinterpret_cast(originatorTy.getOpaqueValue()))); + auto originator = T->getOriginator(); + if (auto *typeVar = originator.dyn_cast()) { + visit(typeVar); + } else if (auto *VD = originator.dyn_cast()) { + Printer << "decl = "; + Printer << VD->getName(); + } else { + visit(originator.get()); + } Printer << ">>"; } else { Printer << "<>"; diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 05e244eff72c4..16dd6df0b45c6 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -7016,7 +7016,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint( // type but a hole. auto shouldRecordFixForHole = [&](HoleType *baseType) { auto *originator = - baseType->getOriginatorType().dyn_cast(); + baseType->getOriginator().dyn_cast(); if (!originator) return false; diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index b78513e0b151a..789db2a2bb485 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -1012,7 +1012,7 @@ sawSolution(const constraints::Solution &S) { return S.simplifyType(completionTy.transform([&](Type type) { if (auto *hole = type->getAs()) { if (auto *typeVar = - hole->getOriginatorType().dyn_cast()) { + hole->getOriginator().dyn_cast()) { if (auto *GP = typeVar->getImpl().getGenericParameter()) { // Code completion depends on generic parameter type being // represented in terms of `ArchetypeType` since it's easy From 17cb2d215257aba4b5f121eada24faefe9cd47d0 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 22 Sep 2020 16:08:40 -0700 Subject: [PATCH 2/3] [ConstraintSystem] Add a new flag which specifies that solver is used for code completion --- lib/Sema/ConstraintSystem.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 1612bfd2a5c11..2a5201f7c47d3 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1293,6 +1293,11 @@ enum class ConstraintSystemFlags { /// Don't try to type check closure bodies, and leave them unchecked. This is /// used for source tooling functionalities. LeaveClosureBodyUnchecked = 0x20, + + /// If set, we are solving specifically to determine the type of a + /// CodeCompletionExpr, and should continue in the presence of errors wherever + /// possible. + ForCodeCompletion = 0x40, }; /// Options that affect the constraint system as a whole. @@ -3057,6 +3062,12 @@ class ConstraintSystem { return Options.contains(ConstraintSystemFlags::ReusePrecheckedType); } + /// Whether we are solving to determine the possible types of a + /// \c CodeCompletionExpr. + bool isForCodeCompletion() const { + return Options.contains(ConstraintSystemFlags::ForCodeCompletion); + } + /// Log and record the application of the fix. Return true iff any /// subsequent solution would be worse than the best known solution. bool recordFix(ConstraintFix *fix, unsigned impact = 1); From bf58b0c08db7641582ed650ae739a3fa15e21444 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 22 Sep 2020 16:25:25 -0700 Subject: [PATCH 3/3] [CSGen] Use special accessor to get a type of `VarDecl` for solver use Solver needs to handle invalid declarations but only in "code completion" mode since declaration in question might not be associated with code completion, otherwise (if constraint generation fails) there is going to be no completion results. --- lib/Sema/CSGen.cpp | 13 +++++++------ lib/Sema/ConstraintSystem.cpp | 18 ++++++++++++++++++ lib/Sema/ConstraintSystem.h | 5 +++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 01b31f5a718db..e61a7f87fa38a 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1241,10 +1241,9 @@ namespace { if (auto *VD = dyn_cast(E->getDecl())) { knownType = CS.getTypeIfAvailable(VD); if (!knownType) - knownType = VD->getType(); + knownType = CS.getVarType(VD); if (knownType) { - assert(!knownType->isHole()); // If the known type has an error, bail out. if (knownType->hasError()) { if (!CS.hasType(E)) @@ -1252,8 +1251,10 @@ namespace { return nullptr; } - // Set the favored type for this expression to the known type. - CS.setFavoredType(E, knownType.getPointer()); + if (!knownType->hasHole()) { + // Set the favored type for this expression to the known type. + CS.setFavoredType(E, knownType.getPointer()); + } } // This can only happen when failure diagnostics is trying @@ -2016,7 +2017,7 @@ namespace { Type externalType; if (param->getTypeRepr()) { - auto declaredTy = param->getType(); + auto declaredTy = CS.getVarType(param); externalType = CS.openUnboundGenericTypes(declaredTy, paramLoc); } else { // Let's allow parameters which haven't been explicitly typed @@ -3884,7 +3885,7 @@ bool ConstraintSystem::generateConstraints( getConstraintLocator(typeRepr)); setType(typeRepr, backingType); - auto propertyType = wrappedVar->getType(); + auto propertyType = getVarType(wrappedVar); if (propertyType->hasError()) return true; diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index c035f4567deca..168cf7f8222e3 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -5213,3 +5213,21 @@ void ConstraintSystem::recordFixedRequirement(ConstraintLocator *reqLocator, std::make_tuple(GP, reqKind, requirementTy.getPointer())); } } + +// Replace any error types encountered with holes. +Type ConstraintSystem::getVarType(const VarDecl *var) { + auto type = var->getType(); + + // If this declaration is used as part of a code completion + // expression, solver needs to glance over the fact that + // it might be invalid to avoid failing constraint generation + // and produce completion results. + if (!isForCodeCompletion()) + return type; + + return type.transform([&](Type type) { + if (!type->is()) + return type; + return HoleType::get(Context, const_cast(var)); + }); +} diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 2a5201f7c47d3..d7e24ecbf94a1 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -2839,6 +2839,11 @@ class ConstraintSystem { return known->second; } + /// Retrieve type type of the given declaration to be used in + /// constraint system, this is better than calling `getType()` + /// directly because it accounts of constraint system flags. + Type getVarType(const VarDecl *var); + /// Cache the type of the expression argument and return that same /// argument. template