Skip to content

Space Engine: check for empty spaces in Space::forConstructor #17684

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 1 commit into from
Jul 3, 2018
Merged
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
76 changes: 15 additions & 61 deletions lib/Sema/TypeCheckSwitchStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,17 @@ namespace {
}
static Space forConstructor(Type T, Identifier H, bool downgrade,
ArrayRef<Space> SP) {
if (llvm::any_of(SP, std::mem_fn(&Space::isEmpty))) {
// A constructor with an unconstructible parameter can never actually
// be used.
return Space();
}
return Space(T, H, downgrade, SP);
}
static Space forConstructor(Type T, Identifier H, bool downgrade,
std::forward_list<Space> SP) {
// No need to filter SP here; this is only used to copy other
// Constructor spaces.
return Space(T, H, downgrade, SP);
}
static Space forBool(bool C) {
Expand Down Expand Up @@ -493,9 +500,11 @@ namespace {
}
}

/// Convenience declaration to make the intersection operation look more
/// symmetric.
static Space intersect(const Space &a, const Space &b, TypeChecker &TC,
const DeclContext *DC) {
return a.intersect(b, TC, DC).simplify(TC, DC);
return a.intersect(b, TC, DC);
}

// Returns the intersection of this space with another. The intersection
Expand Down Expand Up @@ -736,13 +745,12 @@ namespace {
PAIRCASE (SpaceKind::Constructor, SpaceKind::UnknownCase): {
SmallVector<Space, 4> newSubSpaces;
for (auto subSpace : this->getSpaces()) {
auto diff = subSpace.minus(other, TC, DC, minusCount);
if (!diff)
auto nextSpace = subSpace.minus(other, TC, DC, minusCount);
if (!nextSpace)
return None;
auto nextSpace = diff->simplify(TC, DC);
if (nextSpace.isEmpty())
if (nextSpace.getValue().isEmpty())
return Space();
newSubSpaces.push_back(nextSpace);
newSubSpaces.push_back(nextSpace.getValue());
}
return Space::forConstructor(this->getType(), this->getHead(),
this->canDowngradeToWarning(),
Expand Down Expand Up @@ -947,59 +955,6 @@ namespace {
}
}

// For optimization, attempt to simplify a space by removing any empty
// cases and unpacking empty or singular disjunctions where possible.
Space simplify(TypeChecker &TC, const DeclContext *DC) const {
switch (getKind()) {
case SpaceKind::Constructor: {
// If a constructor has no spaces it is an enum without a payload and
// cannot be optimized further.
if (getSpaces().empty()) {
return *this;
}

// Simplify each component subspace. If, after simplification, any
// subspace contains an empty, then the whole space is empty.
SmallVector<Space, 4> simplifiedSpaces;
for (const auto &space : Spaces) {
auto simplified = space.simplify(TC, DC);
if (simplified.isEmpty())
return Space();

simplifiedSpaces.push_back(simplified);
}

return Space::forConstructor(getType(), Head, canDowngradeToWarning(),
simplifiedSpaces);
}
case SpaceKind::Type: {
// If the decomposition of a space is empty, the space is empty.
if (canDecompose(this->getType(), DC)) {
SmallVector<Space, 4> ss;
decompose(TC, DC, this->getType(), ss);
if (ss.empty()) {
return Space();
}
return *this;
} else {
return *this;
}
}
case SpaceKind::Disjunct: {
// Simplify each disjunct.
SmallVector<Space, 4> simplifiedSpaces;
std::transform(Spaces.begin(), Spaces.end(),
std::back_inserter(simplifiedSpaces),
[&](const Space &el){
return el.simplify(TC, DC);
});
return Space::forDisjunct(simplifiedSpaces);
}
default:
return *this;
}
}

static bool isSwift3DowngradeExhaustivityCase(TypeChecker &TC,
const EnumElementDecl *eed){
if (TC.getLangOpts().isSwiftVersionAtLeast(4))
Expand Down Expand Up @@ -1268,7 +1223,7 @@ namespace {
return;
}

auto uncovered = diff->simplify(TC, DC);
auto uncovered = diff.getValue();
if (unknownCase && uncovered.isEmpty()) {
TC.diagnose(unknownCase->getLoc(), diag::redundant_particular_case)
.highlight(unknownCase->getSourceRange());
Expand All @@ -1279,7 +1234,6 @@ namespace {
// that are implicitly frozen.
uncovered = *uncovered.minus(Space::forUnknown(unknownCase == nullptr),
TC, DC, /*&minusCount*/ nullptr);
uncovered = uncovered.simplify(TC, DC);

if (uncovered.isEmpty())
return;
Expand Down