Skip to content

[CSFix] Handle some cases where the same fix appears in multiple solutions #36045

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
Feb 19, 2021
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
24 changes: 24 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,8 @@ class MissingConformance final : public ConstraintFix {

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

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;

static MissingConformance *forRequirement(ConstraintSystem &cs, Type type,
Type protocolType,
ConstraintLocator *locator);
Expand All @@ -489,6 +491,12 @@ class MissingConformance final : public ConstraintFix {
Type getNonConformingType() { return NonConformingType; }

Type getProtocolType() { return ProtocolType; }

bool isEqual(const ConstraintFix *other) const;

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

/// Skip same-type generic requirement constraint,
Expand Down Expand Up @@ -1386,11 +1394,19 @@ class MoveOutOfOrderArgument final : public ConstraintFix {

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

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;

bool isEqual(const ConstraintFix *other) const;

static MoveOutOfOrderArgument *create(ConstraintSystem &cs,
unsigned argIdx,
unsigned prevArgIdx,
ArrayRef<ParamBinding> bindings,
ConstraintLocator *locator);

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

class AllowInaccessibleMember final : public AllowInvalidMemberRef {
Expand Down Expand Up @@ -1512,10 +1528,18 @@ class AllowInvalidRefInKeyPath final : public ConstraintFix {

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

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;

/// Determine whether give reference requires a fix and produce one.
static AllowInvalidRefInKeyPath *
forRef(ConstraintSystem &cs, ValueDecl *member, ConstraintLocator *locator);

bool isEqual(const ConstraintFix *other) const;

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

private:
static AllowInvalidRefInKeyPath *create(ConstraintSystem &cs, RefKind kind,
ValueDecl *member,
Expand Down
74 changes: 74 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,34 @@ bool MissingConformance::diagnose(const Solution &solution, bool asNote) const {
return failure.diagnose(asNote);
}

bool MissingConformance::diagnoseForAmbiguity(
CommonFixesArray commonFixes) const {
auto *primaryFix = commonFixes.front().second->getAs<MissingConformance>();
assert(primaryFix);

if (llvm::all_of(
commonFixes,
[&primaryFix](
const std::pair<const Solution *, const ConstraintFix *> &entry) {
return primaryFix->isEqual(entry.second);
}))
return diagnose(*commonFixes.front().first);

// If the location is the same but there are different requirements
// involved let's not attempt to diagnose that as an ambiguity.
return false;
}

bool MissingConformance::isEqual(const ConstraintFix *other) const {
auto *conformanceFix = other->getAs<MissingConformance>();
if (!conformanceFix)
return false;

return IsContextual == conformanceFix->IsContextual &&
NonConformingType->isEqual(conformanceFix->NonConformingType) &&
ProtocolType->isEqual(conformanceFix->ProtocolType);
}

MissingConformance *
MissingConformance::forContextual(ConstraintSystem &cs, Type type,
Type protocolType,
Expand Down Expand Up @@ -837,6 +865,29 @@ bool MoveOutOfOrderArgument::diagnose(const Solution &solution,
return failure.diagnose(asNote);
}

bool MoveOutOfOrderArgument::diagnoseForAmbiguity(
CommonFixesArray commonFixes) const {
auto *primaryFix =
commonFixes.front().second->getAs<MoveOutOfOrderArgument>();
assert(primaryFix);

if (llvm::all_of(
commonFixes,
[&primaryFix](
const std::pair<const Solution *, const ConstraintFix *> &entry) {
return primaryFix->isEqual(entry.second);
}))
return diagnose(*commonFixes.front().first);

return false;
}

bool MoveOutOfOrderArgument::isEqual(const ConstraintFix *other) const {
auto OoOFix = other->getAs<MoveOutOfOrderArgument>();
return OoOFix ? ArgIdx == OoOFix->ArgIdx && PrevArgIdx == OoOFix->PrevArgIdx
: false;
}

MoveOutOfOrderArgument *MoveOutOfOrderArgument::create(
ConstraintSystem &cs, unsigned argIdx, unsigned prevArgIdx,
ArrayRef<ParamBinding> bindings, ConstraintLocator *locator) {
Expand Down Expand Up @@ -925,6 +976,29 @@ bool AllowInvalidRefInKeyPath::diagnose(const Solution &solution,
llvm_unreachable("covered switch");
}

bool AllowInvalidRefInKeyPath::diagnoseForAmbiguity(
CommonFixesArray commonFixes) const {
auto *primaryFix =
commonFixes.front().second->getAs<AllowInvalidRefInKeyPath>();
assert(primaryFix);

if (llvm::all_of(
commonFixes,
[&primaryFix](
const std::pair<const Solution *, const ConstraintFix *> &entry) {
return primaryFix->isEqual(entry.second);
})) {
return diagnose(*commonFixes.front().first);
}

return false;
}

bool AllowInvalidRefInKeyPath::isEqual(const ConstraintFix *other) const {
auto *refFix = other->getAs<AllowInvalidRefInKeyPath>();
return refFix ? Kind == refFix->Kind && Member == refFix->Member : false;
}

AllowInvalidRefInKeyPath *
AllowInvalidRefInKeyPath::forRef(ConstraintSystem &cs, ValueDecl *member,
ConstraintLocator *locator) {
Expand Down
28 changes: 28 additions & 0 deletions validation-test/Sema/SwiftUI/rdar74447308.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5

// REQUIRES: objc_interop
// REQUIRES: OS=macosx

import SwiftUI

struct Item {
let id = UUID()
let text = ""
}

class ItemList: ObservableObject {
@Published var items = [Item]()
}

struct ContentView: View {
@StateObject var list = ItemList()

var body: some View {
List {
ForEach(list.items) { item in
// expected-error@-1 {{referencing initializer 'init(_:content:)' on 'ForEach' requires that 'Item' conform to 'Identifiable'}}
Text(item.text)
}
}
}
}
20 changes: 20 additions & 0 deletions validation-test/Sema/SwiftUI/sr14093.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5

// REQUIRES: objc_interop
// REQUIRES: OS=macosx

import SwiftUI
import Foundation

struct ContentView: View {
@State private var date = Date()

var body: some View {
Group {
DatePicker("Enter a date", selection: $date, displayedComponents: .date, in: Date())
// expected-error@-1 {{argument 'in' must precede argument 'displayedComponents'}} {{78-90=}} {{52-52=in: Date(), }}
DatePicker("Enter a date", selection: $date, displayedComponents: .date, in: Date() ... Date().addingTimeInterval(100))
// expected-error@-1 {{argument 'in' must precede argument 'displayedComponents'}} {{78-125=}} {{52-52=in: Date() ... Date().addingTimeInterval(100), }}
}
}
}