Skip to content

[CSBindings] If hole originates from code completion token avoid "fixing" it #34118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ using namespace swift;
using namespace constraints;

void ConstraintSystem::PotentialBindings::inferTransitiveBindings(
const ConstraintSystem &cs, llvm::SmallPtrSetImpl<CanType> &existingTypes,
ConstraintSystem &cs, llvm::SmallPtrSetImpl<CanType> &existingTypes,
const llvm::SmallDenseMap<TypeVariableType *,
ConstraintSystem::PotentialBindings>
&inferredBindings) {
Expand Down Expand Up @@ -144,7 +144,7 @@ isUnviableDefaultType(Type defaultType,
}

void ConstraintSystem::PotentialBindings::inferDefaultTypes(
const ConstraintSystem &cs, llvm::SmallPtrSetImpl<CanType> &existingTypes) {
ConstraintSystem &cs, llvm::SmallPtrSetImpl<CanType> &existingTypes) {
auto isDirectRequirement = [&](Constraint *constraint) -> bool {
if (auto *typeVar = constraint->getFirstType()->getAs<TypeVariableType>()) {
auto *repr = cs.getRepresentative(typeVar);
Expand Down Expand Up @@ -300,7 +300,7 @@ void ConstraintSystem::PotentialBindings::inferDefaultTypes(
}

void ConstraintSystem::PotentialBindings::finalize(
const ConstraintSystem &cs,
ConstraintSystem &cs,
const llvm::SmallDenseMap<TypeVariableType *,
ConstraintSystem::PotentialBindings>
&inferredBindings) {
Expand Down Expand Up @@ -377,6 +377,15 @@ void ConstraintSystem::PotentialBindings::finalize(
PotentiallyIncomplete = true;
}

// If this type variable is associated with a code completion token
// and it failed to infer any bindings let's adjust hole's locator
// to point to a code completion token to avoid attempting to "fix"
// this problem since its rooted in the fact that constraint system
// is under-constrained.
if (AssociatedCodeCompletionToken) {
locator = cs.getConstraintLocator(AssociatedCodeCompletionToken);
}

addPotentialBinding(PotentialBinding::forHole(TypeVar, locator));
}

Expand Down Expand Up @@ -620,8 +629,7 @@ bool ConstraintSystem::PotentialBindings::favoredOverDisjunction(
}

ConstraintSystem::PotentialBindings
ConstraintSystem::inferBindingsFor(TypeVariableType *typeVar,
bool finalize) const {
ConstraintSystem::inferBindingsFor(TypeVariableType *typeVar, bool finalize) {
assert(typeVar->getImpl().getRepresentative(nullptr) == typeVar &&
"not a representative");
assert(!typeVar->getImpl().getFixedType(nullptr) && "has a fixed type");
Expand Down Expand Up @@ -769,6 +777,18 @@ ConstraintSystem::getPotentialBindingForRelationalConstraint(

result.InvolvesTypeVariables = true;

// If current type variable is associated with a code completion token
// it's possible that it doesn't have enough contextual information
// to be resolved to anything, so let's note that fact in the potential
// bindings and use it when forming a hole if there are no other bindings
// available.
if (auto *locator = bindingTypeVar->getImpl().getLocator()) {
if (locator->directlyAt<CodeCompletionExpr>()) {
result.AssociatedCodeCompletionToken = locator->getAnchor();
result.PotentiallyIncomplete = true;
}
}

if (constraint->getKind() == ConstraintKind::Subtype &&
kind == AllowedBindingKind::Subtypes) {
result.SubtypeOf.insert(bindingTypeVar);
Expand Down Expand Up @@ -829,7 +849,7 @@ ConstraintSystem::getPotentialBindingForRelationalConstraint(
/// representative type variable, along with flags indicating whether
/// those types should be opened.
bool ConstraintSystem::PotentialBindings::infer(
const ConstraintSystem &cs, llvm::SmallPtrSetImpl<CanType> &exactTypes,
ConstraintSystem &cs, llvm::SmallPtrSetImpl<CanType> &exactTypes,
Constraint *constraint) {
switch (constraint->getKind()) {
case ConstraintKind::Bind:
Expand Down Expand Up @@ -1222,6 +1242,21 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const {
auto *dstLocator = TypeVar->getImpl().getLocator();
auto *srcLocator = Binding.getLocator();

// FIXME: This check could be turned into an assert once
// all code completion kinds are ported to use
// `TypeChecker::typeCheckForCodeCompletion` API.
if (cs.isForCodeCompletion()) {
// If the hole is originated from code completion expression
// let's not try to fix this, anything connected to a
// code completion is allowed to be a hole because presence
// of a code completion token makes constraint system
// under-constrained due to e.g. lack of expressions on the
// right-hand side of the token, which are required for a
// regular type-check.
if (dstLocator->directlyAt<CodeCompletionExpr>())
return None;
}

unsigned defaultImpact = 1;

if (auto *GP = TypeVar->getImpl().getGenericParameter()) {
Expand Down
12 changes: 7 additions & 5 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -4644,6 +4644,8 @@ class ConstraintSystem {
/// `bind param` are present in the system.
bool PotentiallyIncomplete = false;

ASTNode AssociatedCodeCompletionToken = ASTNode();

/// Whether this type variable has literal bindings.
LiteralBindingKind LiteralBinding = LiteralBindingKind::None;

Expand Down Expand Up @@ -4775,25 +4777,25 @@ class ConstraintSystem {
/// \param inferredBindings The set of all bindings inferred for type
/// variables in the workset.
void inferTransitiveBindings(
const ConstraintSystem &cs,
ConstraintSystem &cs,
llvm::SmallPtrSetImpl<CanType> &existingTypes,
const llvm::SmallDenseMap<TypeVariableType *,
ConstraintSystem::PotentialBindings>
&inferredBindings);

/// Infer bindings based on any protocol conformances that have default
/// types.
void inferDefaultTypes(const ConstraintSystem &cs,
void inferDefaultTypes(ConstraintSystem &cs,
llvm::SmallPtrSetImpl<CanType> &existingTypes);

public:
bool infer(const ConstraintSystem &cs,
bool infer(ConstraintSystem &cs,
llvm::SmallPtrSetImpl<CanType> &exactTypes,
Constraint *constraint);

/// Finalize binding computation for this type variable by
/// inferring bindings from context e.g. transitive bindings.
void finalize(const ConstraintSystem &cs,
void finalize(ConstraintSystem &cs,
const llvm::SmallDenseMap<TypeVariableType *,
ConstraintSystem::PotentialBindings>
&inferredBindings);
Expand Down Expand Up @@ -4862,7 +4864,7 @@ class ConstraintSystem {
/// Infer bindings for the given type variable based on current
/// state of the constraint system.
PotentialBindings inferBindingsFor(TypeVariableType *typeVar,
bool finalize = true) const;
bool finalize = true);

private:
Optional<ConstraintSystem::PotentialBinding>
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2991,7 +2991,7 @@ void ConstraintSystem::print(raw_ostream &out) const {
out << " as ";
Type(fixed).print(out, PO);
} else {
inferBindingsFor(tv).dump(out, 1);
const_cast<ConstraintSystem *>(this)->inferBindingsFor(tv).dump(out, 1);
}
} else {
out << " equivalent to ";
Expand Down