Skip to content

[CS] A couple of diagnostic fixes #65651

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 7 commits into from
May 4, 2023
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
25 changes: 25 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,9 @@ enum class FixKind : uint8_t {
/// Produce a warning for a tuple label mismatch.
AllowTupleLabelMismatch,

/// Allow an associated value mismatch for an enum element pattern.
AllowAssociatedValueMismatch,

/// Produce an error for not getting a compile-time constant
NotCompileTimeConst,

Expand Down Expand Up @@ -3238,6 +3241,28 @@ class AllowTupleLabelMismatch final : public ContextualMismatch {
}
};

class AllowAssociatedValueMismatch final : public ContextualMismatch {
AllowAssociatedValueMismatch(ConstraintSystem &cs, Type fromType, Type toType,
ConstraintLocator *locator)
: ContextualMismatch(cs, FixKind::AllowAssociatedValueMismatch, fromType,
toType, locator) {}

public:
std::string getName() const override {
return "allow associated value mismatch";
}

bool diagnose(const Solution &solution, bool asNote = false) const override;

static AllowAssociatedValueMismatch *create(ConstraintSystem &cs,
Type fromType, Type toType,
ConstraintLocator *locator);

static bool classof(const ConstraintFix *fix) {
return fix->getKind() == FixKind::AllowAssociatedValueMismatch;
}
};

class AllowNonOptionalWeak final : public ConstraintFix {
AllowNonOptionalWeak(ConstraintSystem &cs, ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::AllowNonOptionalWeak, locator) {}
Expand Down
9 changes: 7 additions & 2 deletions include/swift/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
#ifndef SWIFT_SEMA_CONSTRAINTLOCATOR_H
#define SWIFT_SEMA_CONSTRAINTLOCATOR_H

#include "swift/Basic/Debug.h"
#include "swift/Basic/LLVM.h"
#include "swift/AST/ASTNode.h"
#include "swift/AST/Type.h"
#include "swift/AST/Types.h"
#include "swift/Basic/Debug.h"
#include "swift/Basic/LLVM.h"
#include "swift/Basic/NullablePtr.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/PointerIntPair.h"
Expand Down Expand Up @@ -313,6 +314,10 @@ class ConstraintLocator : public llvm::FoldingSetNode {
/// branch, and if so, the kind of branch.
Optional<SingleValueStmtBranchKind> isForSingleValueStmtBranch() const;

/// If the locator in question is for a pattern match, returns the pattern,
/// otherwise \c nullptr.
NullablePtr<Pattern> getPatternMatch() const;

/// Returns true if \p locator is ending with either of the following
/// - Member
/// - Member -> KeyPathDynamicMember
Expand Down
22 changes: 21 additions & 1 deletion lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ struct ASTContext::Implementation {
llvm::DenseMap<Type, InOutType*> InOutTypes;
llvm::DenseMap<std::pair<Type, void*>, DependentMemberType *>
DependentMemberTypes;
llvm::DenseMap<void *, PlaceholderType *> PlaceholderTypes;
llvm::DenseMap<Type, DynamicSelfType *> DynamicSelfTypes;
llvm::DenseMap<std::pair<EnumDecl*, Type>, EnumType*> EnumTypes;
llvm::DenseMap<std::pair<StructDecl*, Type>, StructType*> StructTypes;
Expand Down Expand Up @@ -3124,8 +3125,27 @@ Type ErrorType::get(Type originalType) {

Type PlaceholderType::get(ASTContext &ctx, Originator originator) {
assert(originator);
return new (ctx, AllocationArena::Permanent)

auto hasTypeVariables = [&]() -> bool {
if (originator.is<TypeVariableType *>())
return true;

if (auto *depTy = originator.dyn_cast<DependentMemberType *>())
return depTy->hasTypeVariable();

return false;
}();
auto arena = hasTypeVariables ? AllocationArena::ConstraintSolver
: AllocationArena::Permanent;

auto &cache = ctx.getImpl().getArena(arena).PlaceholderTypes;
auto &entry = cache[originator.getOpaqueValue()];
if (entry)
return entry;

entry = new (ctx, arena)
PlaceholderType(ctx, originator, RecursiveTypeProperties::HasPlaceholder);
return entry;
}

BuiltinIntegerType *BuiltinIntegerType::get(BuiltinIntegerWidth BitWidth,
Expand Down
17 changes: 13 additions & 4 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2203,16 +2203,25 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const {
return std::make_pair(fix, /*impact=*/(unsigned)10);
}

if (auto pattern = getAsPattern(dstLocator->getAnchor())) {
if (dstLocator->getPath().size() == 1 &&
dstLocator->isLastElement<LocatorPathElt::PatternDecl>()) {
if (auto pattern = dstLocator->getPatternMatch()) {
if (dstLocator->isLastElement<LocatorPathElt::PatternDecl>()) {
// If this is the pattern in a for loop, and we have a mismatch of the
// element type, then we don't have any useful contextual information
// for the pattern, and can just bind to a hole without needing to penalize
// the solution further.
auto *seqLoc = cs.getConstraintLocator(
dstLocator->getAnchor(), ConstraintLocator::SequenceElementType);
if (cs.hasFixFor(seqLoc,
FixKind::IgnoreCollectionElementContextualMismatch)) {
return None;
}
// Not being able to infer the type of a variable in a pattern binding
// decl is more dramatic than anything that could happen inside the
// expression because we want to preferrably point the diagnostic to a
// part of the expression that caused us to be unable to infer the
// variable's type.
ConstraintFix *fix =
IgnoreUnresolvedPatternVar::create(cs, pattern, dstLocator);
IgnoreUnresolvedPatternVar::create(cs, pattern.get(), dstLocator);
return std::make_pair(fix, /*impact=*/(unsigned)100);
}
}
Expand Down
48 changes: 24 additions & 24 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,10 @@ bool MissingConformanceFailure::diagnoseAsError() {
llvm::SmallPtrSet<Expr *, 4> anchors;
for (const auto *fix : getSolution().Fixes) {
if (auto anchor = fix->getAnchor()) {
if (anchor.is<Expr *>())
auto path = fix->getLocator()->getPath();
SourceRange range;
simplifyLocator(anchor, path, range);
if (anchor && anchor.is<Expr *>())
anchors.insert(getAsExpr(anchor));
}
}
Expand Down Expand Up @@ -654,10 +657,15 @@ bool MissingConformanceFailure::diagnoseAsAmbiguousOperatorRef() {
if (!ODRE)
return false;

auto isStandardType = [](Type ty) {
return ty->isStdlibType() || ty->is<TupleType>();
};

auto name = ODRE->getDecls().front()->getBaseName();
if (!(name.isOperator() && getLHS()->isStdlibType() && getRHS()->isStdlibType()))
if (!(name.isOperator() && isStandardType(getLHS()) &&
isStandardType(getRHS()))) {
return false;

}
// If this is an operator reference and both types are from stdlib,
// let's produce a generic diagnostic about invocation and a note
// about missing conformance just in case.
Expand Down Expand Up @@ -2448,9 +2456,6 @@ bool ContextualFailure::diagnoseAsError() {
return false;
}

if (diagnoseExtraneousAssociatedValues())
return true;

// Special case of some common conversions involving Swift.String
// indexes, catching cases where people attempt to index them with an integer.
if (isIntegerToStringIndexConversion()) {
Expand Down Expand Up @@ -2891,24 +2896,6 @@ void ContextualFailure::tryFixIts(InFlightDiagnostic &diagnostic) const {
return;
}

bool ContextualFailure::diagnoseExtraneousAssociatedValues() const {
if (auto match =
getLocator()->getLastElementAs<LocatorPathElt::PatternMatch>()) {
if (auto enumElementPattern =
dyn_cast<EnumElementPattern>(match->getPattern())) {
emitDiagnosticAt(enumElementPattern->getNameLoc(),
diag::enum_element_pattern_assoc_values_mismatch,
enumElementPattern->getName());
emitDiagnosticAt(enumElementPattern->getNameLoc(),
diag::enum_element_pattern_assoc_values_remove)
.fixItRemove(enumElementPattern->getSubPattern()->getSourceRange());
return true;
}
}

return false;
}

bool ContextualFailure::diagnoseCoercionToUnrelatedType() const {
auto anchor = getRawAnchor();
auto *coerceExpr = getAsExpr<CoerceExpr>(anchor);
Expand Down Expand Up @@ -8684,6 +8671,19 @@ bool TupleLabelMismatchWarning::diagnoseAsError() {
return true;
}

bool AssociatedValueMismatchFailure::diagnoseAsError() {
auto match = getLocator()->castLastElementTo<LocatorPathElt::PatternMatch>();
auto *enumElementPattern = dyn_cast<EnumElementPattern>(match.getPattern());

emitDiagnosticAt(enumElementPattern->getNameLoc(),
diag::enum_element_pattern_assoc_values_mismatch,
enumElementPattern->getName());
emitDiagnosticAt(enumElementPattern->getNameLoc(),
diag::enum_element_pattern_assoc_values_remove)
.fixItRemove(enumElementPattern->getSubPattern()->getSourceRange());
return true;
}

bool SwiftToCPointerConversionInInvalidContext::diagnoseAsError() {
auto argInfo = getFunctionArgApplyInfo(getLocator());
if (!argInfo)
Expand Down
13 changes: 9 additions & 4 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -715,10 +715,6 @@ class ContextualFailure : public FailureDiagnostic {
/// Diagnose failed conversion in a `CoerceExpr`.
bool diagnoseCoercionToUnrelatedType() const;

/// Diagnose cases where a pattern tried to match associated values but
/// the enum case had none.
bool diagnoseExtraneousAssociatedValues() const;

/// Produce a specialized diagnostic if this is an invalid conversion to Bool.
bool diagnoseConversionToBool() const;

Expand Down Expand Up @@ -2813,6 +2809,15 @@ class TupleLabelMismatchWarning final : public ContextualFailure {
bool diagnoseAsError() override;
};

class AssociatedValueMismatchFailure final : public ContextualFailure {
public:
AssociatedValueMismatchFailure(const Solution &solution, Type fromType,
Type toType, ConstraintLocator *locator)
: ContextualFailure(solution, fromType, toType, locator) {}

bool diagnoseAsError() override;
};

/// Diagnose situations where Swift -> C pointer implicit conversion
/// is attempted on a Swift function instead of one imported from C header.
///
Expand Down
21 changes: 18 additions & 3 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2139,9 +2139,10 @@ IgnoreResultBuilderWithReturnStmts::create(ConstraintSystem &cs, Type builderTy,

bool IgnoreUnresolvedPatternVar::diagnose(const Solution &solution,
bool asNote) const {
// Not being able to infer the type of a pattern should already have been
// diagnosed on the pattern's initializer or as a structural issue of the AST.
return true;
// An unresolved AnyPatternDecl means there was some issue in the match
// that means we couldn't infer the pattern. We don't have a diagnostic to
// emit here, the failure should be diagnosed by the fix for expression.
return false;
}

IgnoreUnresolvedPatternVar *
Expand Down Expand Up @@ -2426,6 +2427,20 @@ bool AllowTupleLabelMismatch::diagnose(const Solution &solution,
return warning.diagnose(asNote);
}

AllowAssociatedValueMismatch *
AllowAssociatedValueMismatch::create(ConstraintSystem &cs, Type fromType,
Type toType, ConstraintLocator *locator) {
return new (cs.getAllocator())
AllowAssociatedValueMismatch(cs, fromType, toType, locator);
}

bool AllowAssociatedValueMismatch::diagnose(const Solution &solution,
bool asNote) const {
AssociatedValueMismatchFailure failure(solution, getFromType(), getToType(),
getLocator());
return failure.diagnose(asNote);
}

bool AllowSwiftToCPointerConversion::diagnose(const Solution &solution,
bool asNote) const {
SwiftToCPointerConversionInInvalidContext failure(solution, getLocator());
Expand Down
12 changes: 9 additions & 3 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2471,6 +2471,9 @@ namespace {
: nullptr;
};

auto matchLoc =
locator.withPathElement(LocatorPathElt::PatternMatch(pattern));

// Always prefer a contextual type when it's available.
if (externalPatternType) {
type = externalPatternType;
Expand All @@ -2479,8 +2482,8 @@ namespace {
type = CS.getType(initializer)->getRValueType();
} else {
type = CS.createTypeVariable(
CS.getConstraintLocator(pattern,
ConstraintLocator::AnyPatternDecl),
CS.getConstraintLocator(matchLoc,
LocatorPathElt::AnyPatternDecl()),
TVO_CanBindToNoEscape | TVO_CanBindToHole);
}
return setType(type);
Expand Down Expand Up @@ -2515,9 +2518,12 @@ namespace {
}
}

auto matchLoc =
locator.withPathElement(LocatorPathElt::PatternMatch(pattern));

if (!varType) {
varType = CS.createTypeVariable(
CS.getConstraintLocator(pattern,
CS.getConstraintLocator(matchLoc,
LocatorPathElt::NamedPatternDecl()),
TVO_CanBindToNoEscape | TVO_CanBindToHole);

Expand Down
Loading