Skip to content
Permalink
Browse files
"Sober up" the type checker by improving type inference over dictionary
literals that lack a contextual type.

This fixes SR-305
  • Loading branch information
jopamer committed Apr 27, 2016
1 parent 2351085 commit 2cdd7d64e1e2add7bcfd5452d36e7f5fc6c86a03
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 15 deletions.
@@ -69,6 +69,22 @@ static bool isArithmeticOperatorDecl(ValueDecl *vd) {
vd->getName().str() == "%");
}

static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS,
TypeVariableType* tyvar1,
TypeVariableType* tyvar2) {
if (tyvar1 && tyvar2) {
auto rep1 = CS.getRepresentative(tyvar1);
auto rep2 = CS.getRepresentative(tyvar2);

if (rep1 != rep2) {
CS.mergeEquivalenceClasses(rep1, rep2, /*updateWorkList*/ false);
return true;
}
}

return false;
}

namespace {

/// Internal struct for tracking information about types within a series
@@ -327,14 +343,7 @@ namespace {
auto tyvar1 = acp1->getType()->getAs<TypeVariableType>();
auto tyvar2 = acp2->getType()->getAs<TypeVariableType>();

if (tyvar1 && tyvar2) {
auto rep1 = CS.getRepresentative(tyvar1);
auto rep2 = CS.getRepresentative(tyvar2);

if (rep1 != rep2) {
CS.mergeEquivalenceClasses(rep1, rep2, /*updateWorkList*/ false);
}
}
mergeRepresentativeEquivalenceClasses(CS, tyvar1, tyvar2);
}
}
}
@@ -1859,6 +1868,12 @@ namespace {
return arrayTy;
}

static bool isMergeableValueKind(Expr *expr) {
return isa<CollectionExpr>(expr) ||
isa<StringLiteralExpr>(expr) || isa<IntegerLiteralExpr>(expr) ||
isa<FloatLiteralExpr>(expr);
}

Type visitDictionaryExpr(DictionaryExpr *expr) {
ASTContext &C = CS.getASTContext();
// A dictionary expression can be of a type T that conforms to the
@@ -1908,16 +1923,67 @@ namespace {
TupleTypeElt(dictionaryValueTy) };
Type elementTy = TupleType::get(tupleElts, C);

// Keep track of which elements have been "merged". This way, we won't create
// needless conversion constraints for elements whose equivalence classes have
// been merged.
llvm::DenseSet<Expr *> mergedElements;

// If no contextual type is present, Merge equivalence classes of key
// and value types as necessary.
if (!CS.getContextualType(expr)) {
for (auto element1 : expr->getElements()) {
for (auto element2 : expr->getElements()) {
if (element1 == element2)
continue;

auto tty1 = element1->getType()->getAs<TupleType>();
auto tty2 = element2->getType()->getAs<TupleType>();

if (tty1 && tty2) {
auto mergedKey = false;
auto mergedValue = false;

auto keyTyvar1 = tty1->getElementTypes()[0]->
getAs<TypeVariableType>();
auto keyTyvar2 = tty2->getElementTypes()[0]->
getAs<TypeVariableType>();

mergedKey = mergeRepresentativeEquivalenceClasses(CS,
keyTyvar1, keyTyvar2);

auto valueTyvar1 = tty1->getElementTypes()[1]->
getAs<TypeVariableType>();
auto valueTyvar2 = tty2->getElementTypes()[1]->
getAs<TypeVariableType>();

auto elemExpr1 = dyn_cast<TupleExpr>(element1)->getElements()[1];
auto elemExpr2 = dyn_cast<TupleExpr>(element2)->getElements()[1];

if (elemExpr1->getKind() == elemExpr2->getKind() &&
isMergeableValueKind(elemExpr1)) {
mergedValue = mergeRepresentativeEquivalenceClasses(CS,
valueTyvar1, valueTyvar2);
}

if (mergedKey && mergedValue)
mergedElements.insert(element2);
}
}
}
}

// Introduce conversions from each element to the element type of the
// dictionary.
// dictionary. (If the equivalence class of an element has already been
// merged with a previous one, skip it.)
unsigned index = 0;
for (auto element : expr->getElements()) {
CS.addConstraint(ConstraintKind::Conversion,
element->getType(),
elementTy,
CS.getConstraintLocator(
expr,
LocatorPathElt::getTupleElement(index++)));
if (!mergedElements.count(element))
CS.addConstraint(ConstraintKind::Conversion,
element->getType(),
elementTy,
CS.getConstraintLocator(
expr,
LocatorPathElt::getTupleElement(index++)));
}

return dictionaryTy;
@@ -0,0 +1,26 @@
// RUN: %target-parse-verify-swift
let dict = [
"keys": [
"key 1": ["key": "value"],
"key 2": ["key": "value"],
"key 3": ["key": "value"],
"key 4": ["key": "value"],
"key 5": ["key": "value"],
"key 6": ["key": "value"],
"key 7": ["key": "value"],
"key 8": ["key": "value"],
"key 9": ["key": "value"],
"key 10": ["key": "value"],
"key 11": ["key": "value"],
"key 12": ["key": "value"],
"key 13": ["key": "value"],
"key 14": ["key": "value"],
"key 15": ["key": "value"],
"key 16": ["key": "value"],
"key 17": ["key": "value"],
"key 18": ["key": "value"],
"key 19": ["key": "value"],
"key 20": ["key": "value"],
]
]

This comment has been minimized.

Copy link
@mikeger

mikeger Apr 27, 2016

Missing newline at the end of file

7 comments on commit 2cdd7d6

@dduan
Copy link
Collaborator

@dduan dduan commented on 2cdd7d6 Apr 27, 2016

Choose a reason for hiding this comment

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

Love the commit message!

@slavapestov
Copy link
Member

Choose a reason for hiding this comment

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

From reddit post to fix in less than 12 hours -- nice work @jopamer!

@lattner
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

@pegasd
Copy link

@pegasd pegasd commented on 2cdd7d6 May 2, 2016

Choose a reason for hiding this comment

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

Awesome!

@dipkasyap
Copy link

@dipkasyap dipkasyap commented on 2cdd7d6 May 22, 2017

Choose a reason for hiding this comment

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

Xcode 8.2 , Swift 3.0 , Plateform : IOS, Device Type: Simulator

This seems have another face in terms of "Nil Coalescing Operator". Compilation was that much slow as i can refer compilation time was too much long, And the main part not only compilation slows down, after compilation execution also slowed down, for example simple screen transaction of screen of performing scrolling on UITableView also Slowed down.

As i was trying to construct dictionary like

let dict = [
"key1" = modalobject.someValue 1?? "default value",
"key2" = modalobject.someValue 2?? "default value",
"key3" = modalobject.someValue3 ?? "default value",
"key4" = modalobject.someValue4 ?? "default value"

]

@CodaFi
Copy link
Member

@CodaFi CodaFi commented on 2cdd7d6 May 22, 2017

Choose a reason for hiding this comment

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

@dipkasyap please, please, please do not report bugs under ancient commits. Please go file an SR at bugs.swift.org.

@dipkasyap
Copy link

Choose a reason for hiding this comment

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

@CodaFi , Sorry , i was unaware of that, my mistake ! Any way thanks a lot for valuable suggestion.

Please sign in to comment.