Skip to content

[ConstraintSystem] Remove implementation of operator designated types in the solver. #34315

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 2 commits into from
Oct 15, 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
4 changes: 0 additions & 4 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,6 @@ namespace swift {
/// Disable constraint system performance hacks.
bool DisableConstraintSolverPerformanceHacks = false;

/// Enable constraint solver support for experimental
/// operator protocol designator feature.
bool SolverEnableOperatorDesignatedTypes = false;

/// Enable experimental support for one-way constraints for the
/// parameters of closures.
bool EnableOneWayClosureParameters = false;
Expand Down
4 changes: 0 additions & 4 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,6 @@ def enable_operator_designated_types :
Flag<["-"], "enable-operator-designated-types">,
HelpText<"Enable operator designated types">;

def solver_enable_operator_designated_types :
Flag<["-"], "solver-enable-operator-designated-types">,
HelpText<"Enable operator designated types in constraint solver">;

def enable_invalid_ephemeralness_as_error :
Flag<["-"], "enable-invalid-ephemeralness-as-error">,
HelpText<"Diagnose invalid ephemeral to non-ephemeral conversions as errors">;
Expand Down
15 changes: 0 additions & 15 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -5032,8 +5032,6 @@ class ConstraintSystem {
/// \returns The selected disjunction.
Constraint *selectDisjunction();

Constraint *selectApplyDisjunction();

/// Solve the system of constraints generated from provided expression.
///
/// \param target The target to generate constraints from.
Expand Down Expand Up @@ -5293,19 +5291,6 @@ class ConstraintSystem {
typedef std::function<void(SmallVectorImpl<unsigned> &options)>
PartitionAppendCallback;

// Attempt to sort nominalTypes based on what we can discover about
// calls into the overloads in the disjunction that bindOverload is
// a part of.
void sortDesignatedTypes(SmallVectorImpl<NominalTypeDecl *> &nominalTypes,
Constraint *bindOverload);

// Partition the choices in a disjunction based on those that match
// the designated types for the operator that the disjunction was
// formed for.
void partitionForDesignatedTypes(ArrayRef<Constraint *> Choices,
ConstraintMatchLoop forEachChoice,
PartitionAppendCallback appendPartition);

// Partition the choices in the disjunction into groups that we will
// iterate over in an order appropriate to attempt to stop before we
// have to visit all of the options.
Expand Down
2 changes: 0 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,6 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args,
// Always enable operator designated types for the standard library.
Opts.EnableOperatorDesignatedTypes |= FrontendOpts.ParseStdlib;

Opts.SolverEnableOperatorDesignatedTypes |=
Args.hasArg(OPT_solver_enable_operator_designated_types);
Opts.EnableOneWayClosureParameters |=
Args.hasArg(OPT_experimental_one_way_closure_params);

Expand Down
202 changes: 1 addition & 201 deletions lib/Sema/CSSolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1929,30 +1929,6 @@ Constraint *ConstraintSystem::getUnboundBindOverloadDisjunction(
return result->first;
}

// Find a disjunction associated with an ApplicableFunction constraint
// where we have some information about all of the types of in the
// function application (even if we only know something about what the
// types conform to and not actually a concrete type).
Constraint *ConstraintSystem::selectApplyDisjunction() {
for (auto &constraint : InactiveConstraints) {
if (constraint.getKind() != ConstraintKind::ApplicableFunction)
continue;

auto *applicable = &constraint;
if (haveTypeInformationForAllArguments(
applicable->getFirstType()->castTo<FunctionType>())) {
auto *tyvar = applicable->getSecondType()->castTo<TypeVariableType>();

// If we have created the disjunction for this apply, find it.
auto *disjunction = getUnboundBindOverloadDisjunction(tyvar);
if (disjunction)
return disjunction;
}
}

return nullptr;
}

static bool isOperatorBindOverload(Constraint *bindOverload) {
if (bindOverload->getKind() != ConstraintKind::BindOverload)
return false;
Expand All @@ -1965,165 +1941,6 @@ static bool isOperatorBindOverload(Constraint *bindOverload) {
return funcDecl && funcDecl->getOperatorDecl();
}

// Given a bind overload constraint for an operator, return the
// protocol designated as the first place to look for overloads of the
// operator.
static ArrayRef<NominalTypeDecl *>
getOperatorDesignatedNominalTypes(Constraint *bindOverload) {
auto choice = bindOverload->getOverloadChoice();
auto *funcDecl = cast<FuncDecl>(choice.getDecl());
auto *operatorDecl = funcDecl->getOperatorDecl();
return operatorDecl->getDesignatedNominalTypes();
}

void ConstraintSystem::sortDesignatedTypes(
SmallVectorImpl<NominalTypeDecl *> &nominalTypes,
Constraint *bindOverload) {
auto *tyvar = bindOverload->getFirstType()->castTo<TypeVariableType>();
auto applicableFns = getConstraintGraph().gatherConstraints(
tyvar, ConstraintGraph::GatheringKind::EquivalenceClass,
[](Constraint *match) {
return match->getKind() == ConstraintKind::ApplicableFunction;
});

// FIXME: This is not true when we run the constraint optimizer.
// assert(applicableFns.size() <= 1);

// We have a disjunction for an operator but no application of it,
// so it's being passed as an argument.
if (applicableFns.size() == 0)
return;

// FIXME: We have more than one applicable per disjunction as a
// result of merging disjunction type variables. We may want
// to rip that out at some point.
Constraint *foundApplicable = nullptr;
SmallVector<Optional<Type>, 2> argumentTypes;
for (auto *applicableFn : applicableFns) {
argumentTypes.clear();
auto *fnTy = applicableFn->getFirstType()->castTo<FunctionType>();
ArgumentInfoCollector argInfo(*this, fnTy);
// Stop if we hit anything with concrete types or conformances to
// literals.
if (!argInfo.getTypes().empty() || !argInfo.getLiteralProtocols().empty()) {
foundApplicable = applicableFn;
break;
}
}

if (!foundApplicable)
return;

// FIXME: It would be good to avoid this redundancy.
auto *fnTy = foundApplicable->getFirstType()->castTo<FunctionType>();
ArgumentInfoCollector argInfo(*this, fnTy);

size_t nextType = 0;
for (auto argType : argInfo.getTypes()) {
auto *nominal = argType->getAnyNominal();
for (size_t i = nextType; i < nominalTypes.size(); ++i) {
if (nominal == nominalTypes[i]) {
std::swap(nominalTypes[nextType], nominalTypes[i]);
++nextType;
break;
} else if (auto *protoDecl = dyn_cast<ProtocolDecl>(nominalTypes[i])) {
if (TypeChecker::conformsToProtocol(argType, protoDecl, DC)) {
std::swap(nominalTypes[nextType], nominalTypes[i]);
++nextType;
break;
}
}
}
}

if (nextType + 1 >= nominalTypes.size())
return;

for (auto *protocol : argInfo.getLiteralProtocols()) {
auto defaultType = TypeChecker::getDefaultType(protocol, DC);
// ExpressibleByNilLiteral does not have a default type.
if (!defaultType)
continue;
auto *nominal = defaultType->getAnyNominal();
for (size_t i = nextType + 1; i < nominalTypes.size(); ++i) {
if (nominal == nominalTypes[i]) {
std::swap(nominalTypes[nextType], nominalTypes[i]);
++nextType;
break;
}
}
}
}

void ConstraintSystem::partitionForDesignatedTypes(
ArrayRef<Constraint *> Choices, ConstraintMatchLoop forEachChoice,
PartitionAppendCallback appendPartition) {

auto types = getOperatorDesignatedNominalTypes(Choices[0]);
if (types.empty())
return;

SmallVector<NominalTypeDecl *, 4> designatedNominalTypes(types.begin(),
types.end());

if (designatedNominalTypes.size() > 1)
sortDesignatedTypes(designatedNominalTypes, Choices[0]);

SmallVector<SmallVector<unsigned, 4>, 4> definedInDesignatedType;
SmallVector<SmallVector<unsigned, 4>, 4> definedInExtensionOfDesignatedType;

auto examineConstraint =
[&](unsigned constraintIndex, Constraint *constraint) -> bool {
auto *decl = constraint->getOverloadChoice().getDecl();
auto *funcDecl = cast<FuncDecl>(decl);

auto *parentDC = funcDecl->getParent();
auto *parentDecl = parentDC->getSelfNominalTypeDecl();

// Skip anything not defined in a nominal type.
if (!parentDecl)
return false;

for (auto designatedTypeIndex : indices(designatedNominalTypes)) {
auto *designatedNominal =
designatedNominalTypes[designatedTypeIndex];

if (parentDecl != designatedNominal)
continue;

auto &constraints =
isa<ExtensionDecl>(parentDC)
? definedInExtensionOfDesignatedType[designatedTypeIndex]
: definedInDesignatedType[designatedTypeIndex];

constraints.push_back(constraintIndex);
return true;
}

return false;
};

definedInDesignatedType.resize(designatedNominalTypes.size());
definedInExtensionOfDesignatedType.resize(designatedNominalTypes.size());

forEachChoice(Choices, examineConstraint);

// Now collect the overload choices that are defined within the type
// that was designated in the operator declaration.
// Add partitions for each of the overloads we found in types that
// were designated as part of the operator declaration.
for (auto designatedTypeIndex : indices(designatedNominalTypes)) {
if (designatedTypeIndex < definedInDesignatedType.size()) {
auto &primary = definedInDesignatedType[designatedTypeIndex];
appendPartition(primary);
}
if (designatedTypeIndex < definedInExtensionOfDesignatedType.size()) {
auto &secondary = definedInExtensionOfDesignatedType[designatedTypeIndex];
appendPartition(secondary);
}
}
}

// Performance hack: if there are two generic overloads, and one is
// more specialized than the other, prefer the more-specialized one.
static Constraint *tryOptimizeGenericDisjunction(
Expand Down Expand Up @@ -2266,8 +2083,7 @@ void ConstraintSystem::partitionDisjunction(
}

// Partition SIMD operators.
if (!getASTContext().TypeCheckerOpts.SolverEnableOperatorDesignatedTypes &&
isOperatorBindOverload(Choices[0])) {
if (isOperatorBindOverload(Choices[0])) {
forEachChoice(Choices, [&](unsigned index, Constraint *constraint) -> bool {
if (!isOperatorBindOverload(constraint))
return false;
Expand All @@ -2291,11 +2107,6 @@ void ConstraintSystem::partitionDisjunction(
}
};

if (getASTContext().TypeCheckerOpts.SolverEnableOperatorDesignatedTypes &&
isOperatorBindOverload(Choices[0])) {
partitionForDesignatedTypes(Choices, forEachChoice, appendPartition);
}

SmallVector<unsigned, 4> everythingElse;
// Gather the remaining options.
forEachChoice(Choices, [&](unsigned index, Constraint *constraint) -> bool {
Expand All @@ -2320,17 +2131,6 @@ Constraint *ConstraintSystem::selectDisjunction() {
if (disjunctions.empty())
return nullptr;

// Attempt apply disjunctions first. When we have operators with
// designated types, this is important, because it allows us to
// select all the preferred operator overloads prior to other
// disjunctions that we may not be able to short-circuit, allowing
// us to eliminate behavior that is exponential in the number of
// operators in the expression.
if (getASTContext().TypeCheckerOpts.SolverEnableOperatorDesignatedTypes) {
if (auto *disjunction = selectApplyDisjunction())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think selectApplyDisjunction could also be removed now right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you're right, thanks for catching that!

return disjunction;
}

if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions))
return disjunction;

Expand Down
3 changes: 1 addition & 2 deletions lib/Sema/CSStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,7 @@ bool DisjunctionStep::shortCircuitDisjunctionAt(
if (currentChoice->getKind() == ConstraintKind::BindOverload &&
isSIMDOperator(currentChoice->getOverloadChoice().getDecl()) &&
lastSuccessfulChoice->getKind() == ConstraintKind::BindOverload &&
!isSIMDOperator(lastSuccessfulChoice->getOverloadChoice().getDecl()) &&
!ctx.TypeCheckerOpts.SolverEnableOperatorDesignatedTypes) {
!isSIMDOperator(lastSuccessfulChoice->getOverloadChoice().getDecl())) {
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/add_with_nil.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift -swift-version 5 -solver-enable-operator-designated-types -solver-disable-shrink -disable-constraint-solver-performance-hacks
// RUN: %target-typecheck-verify-swift -swift-version 5

func test(_ x: Int) -> Int {
return x + nil
Expand Down
4 changes: 2 additions & 2 deletions test/attr/attr_implements_fp.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// RUN: %empty-directory(%t)
// RUN: echo 'main()' >%t/main.swift
// RUN: %target-swiftc_driver -o %t/a.out %s %t/main.swift -Xfrontend -enable-operator-designated-types -Xfrontend -solver-enable-operator-designated-types
// RUN: %target-swiftc_driver -o %t/a.out %s %t/main.swift
// RUN: %target-codesign %t/a.out
// RUN: %target-run %t/a.out | %FileCheck %s
// REQUIRES: executable_test
Expand All @@ -13,7 +13,7 @@
public var comparedAsCauxmparablesCount : Int = 0
public var comparedAsFauxtsCount : Int = 0

infix operator .< : ComparisonPrecedence, BinaryFauxtingPoint, Cauxmparable
infix operator .< : ComparisonPrecedence

public protocol Cauxmparable {
static func .< (lhs: Self, rhs: Self) -> Bool
Expand Down
2 changes: 1 addition & 1 deletion test/attr/attr_implements_serial.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// RUN: %empty-directory(%t)
// RUN: echo 'client()' >%t/main.swift
// RUN: %target-build-swift-dylib(%t/%target-library-name(AttrImplFP)) -module-name AttrImplFP -emit-module -emit-module-path %t/AttrImplFP.swiftmodule %S/attr_implements_fp.swift -Xfrontend -enable-operator-designated-types -Xfrontend -solver-enable-operator-designated-types
// RUN: %target-build-swift-dylib(%t/%target-library-name(AttrImplFP)) -module-name AttrImplFP -emit-module -emit-module-path %t/AttrImplFP.swiftmodule %S/attr_implements_fp.swift
// RUN: %target-build-swift -I %t -o %t/a.out %s %t/main.swift -L %t %target-rpath(%t) -lAttrImplFP
// RUN: %target-codesign %t/a.out
// RUN: %target-codesign %t/%target-library-name(AttrImplFP)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// RUN: %target-typecheck-verify-swift -swift-version 4 -solver-expression-time-threshold=1 -solver-disable-shrink -disable-constraint-solver-performance-hacks -solver-enable-operator-designated-types
// RUN: %target-typecheck-verify-swift -swift-version 4 -solver-expression-time-threshold=1
// REQUIRES: tools-release,no_asan

func test(_ i: Int, _ j: Int) -> Int {
return 1 + (((i >> 1) + (i >> 2) + (i >> 3) + (i >> 4) << 1) << 1) & 0x40 +
1 + (((i >> 1) + (i >> 2) + (i >> 3) + (i >> 4) << 1) << 1) & 0x40 +
1 + (((i >> 1) + (i >> 2) + (i >> 3) + (i >> 4) << 1) << 1) & 0x40
// expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// RUN: %target-typecheck-verify-swift -swift-version 5 -solver-enable-operator-designated-types -solver-disable-shrink -disable-constraint-solver-performance-hacks
// RUN: %target-typecheck-verify-swift -swift-version 5 -solver-expression-time-threshold=1

// rdar://problem/32998180
func checksum(value: UInt16) -> UInt16 {
var checksum = (((value >> 2) ^ (value >> 8) ^ (value >> 12) ^ (value >> 14)) & 0x01) << 1
// expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}}
checksum |= (((value) ^ (value >> UInt16(4)) ^ (value >> UInt16(6)) ^ (value >> UInt16(10))) & 0x01)
checksum ^= 0x02
return checksum
Expand All @@ -17,4 +18,5 @@ func f(tail: UInt64, byteCount: UInt64) {
func size(count: Int) {
// Size of the buffer we need to allocate
let _ = count * MemoryLayout<Float>.size * (4 + 3 + 3 + 2 + 4)
// expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink -disable-constraint-solver-performance-hacks -solver-enable-operator-designated-types
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1
// REQUIRES: tools-release,no_asan

let i: Int? = 1
let j: Int?
let k: Int? = 2

// expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}}
let _ = [i, j, k].reduce(0 as Int?) {
$0 != nil && $1 != nil ? $0! + $1! : ($0 != nil ? $0! : ($1 != nil ? $1! : nil))
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink -disable-constraint-solver-performance-hacks -solver-enable-operator-designated-types
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1
// REQUIRES: tools-release,no_asan

func test(strings: [String]) {
for string in strings {
let _ = string.split(omittingEmptySubsequences: false) { $0 == "C" || $0 == "D" || $0 == "H" || $0 == "S"}
// expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink -disable-constraint-solver-performance-hacks -solver-enable-operator-designated-types
// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1
// REQUIRES: tools-release,no_asan

// expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}}
let _: (Character) -> Bool = { c in
("a" <= c && c <= "z") || ("A" <= c && c <= "Z") || c == "_"
}
Loading