Skip to content
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
2 changes: 2 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ class RelabelArguments final

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

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;

static RelabelArguments *create(ConstraintSystem &cs,
llvm::ArrayRef<Identifier> correctLabels,
ConstraintLocator *locator);
Expand Down
40 changes: 30 additions & 10 deletions lib/Sema/CSClosure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ Expr *getVoidExpr(ASTContext &ctx) {

/// Find any type variable references inside of an AST node.
class TypeVariableRefFinder : public ASTWalker {
/// A stack of all closures the walker encountered so far.
SmallVector<DeclContext *> ClosureDCs;

ConstraintSystem &CS;
ASTNode Parent;

Expand All @@ -46,9 +49,16 @@ class TypeVariableRefFinder : public ASTWalker {
TypeVariableRefFinder(
ConstraintSystem &cs, ASTNode parent,
llvm::SmallPtrSetImpl<TypeVariableType *> &referencedVars)
: CS(cs), Parent(parent), ReferencedVars(referencedVars) {}
: CS(cs), Parent(parent), ReferencedVars(referencedVars) {
if (auto *closure = getAsExpr<ClosureExpr>(Parent))
ClosureDCs.push_back(closure);
}

std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
if (auto *closure = dyn_cast<ClosureExpr>(expr)) {
ClosureDCs.push_back(closure);
}

if (auto *DRE = dyn_cast<DeclRefExpr>(expr)) {
auto *decl = DRE->getDecl();

Expand Down Expand Up @@ -81,20 +91,33 @@ class TypeVariableRefFinder : public ASTWalker {
return {true, expr};
}

Expr *walkToExprPost(Expr *expr) override {
if (auto *closure = dyn_cast<ClosureExpr>(expr)) {
ClosureDCs.pop_back();
}
return expr;
}

std::pair<bool, Stmt *> walkToStmtPre(Stmt *stmt) override {
// Return statements have to reference outside result type
// since all of them are joined by it if it's not specified
// explicitly.
if (isa<ReturnStmt>(stmt)) {
if (auto *closure = getAsExpr<ClosureExpr>(Parent)) {
inferVariables(CS.getClosureType(closure)->getResult());
// Return is only viable if it belongs to a parent closure.
if (currentClosureDC() == closure)
inferVariables(CS.getClosureType(closure)->getResult());
}
}

return {true, stmt};
}

private:
DeclContext *currentClosureDC() const {
return ClosureDCs.empty() ? nullptr : ClosureDCs.back();
}

void inferVariables(Type type) {
type = type->getWithoutSpecifierType();
// Record the type variable itself because it has to
Expand Down Expand Up @@ -440,7 +463,7 @@ class ClosureConstraintGenerator

cs.addConstraint(
ConstraintKind::Conversion, elementType, initType,
cs.getConstraintLocator(contextualLocator,
cs.getConstraintLocator(sequenceLocator,
ConstraintLocator::SequenceElementType));

// Reference the makeIterator witness.
Expand All @@ -449,9 +472,10 @@ class ClosureConstraintGenerator

Type makeIteratorType =
cs.createTypeVariable(locator, TVO_CanBindToNoEscape);
cs.addValueWitnessConstraint(LValueType::get(sequenceType), makeIterator,
makeIteratorType, closure,
FunctionRefKind::Compound, contextualLocator);
cs.addValueWitnessConstraint(
LValueType::get(sequenceType), makeIterator, makeIteratorType,
closure, FunctionRefKind::Compound,
cs.getConstraintLocator(sequenceLocator, ConstraintLocator::Witness));

// After successful constraint generation, let's record
// solution application target with all relevant information.
Expand Down Expand Up @@ -1176,10 +1200,6 @@ class ClosureConstraintApplication
if (isa<IfConfigDecl>(decl))
return;

// Variable declaration would be handled by a pattern binding.
if (isa<VarDecl>(decl))
return;

// Generate constraints for pattern binding declarations.
if (auto patternBinding = dyn_cast<PatternBindingDecl>(decl)) {
SolutionApplicationTarget target(patternBinding);
Expand Down
6 changes: 6 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,12 @@ class MissingMemberFailure : public InvalidMemberRefFailure {
: InvalidMemberRefFailure(solution, baseType, memberName, locator) {}

SourceLoc getLoc() const override {
auto *locator = getLocator();

if (locator->findLast<LocatorPathElt::ClosureBodyElement>()) {
return constraints::getLoc(getAnchor());
}

// Diagnostic should point to the member instead of its base expression.
return constraints::getLoc(getRawAnchor());
}
Expand Down
32 changes: 32 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,38 @@ bool RelabelArguments::diagnose(const Solution &solution, bool asNote) const {
return failure.diagnose(asNote);
}

bool RelabelArguments::diagnoseForAmbiguity(
CommonFixesArray commonFixes) const {
SmallPtrSet<ValueDecl *, 4> overloadChoices;

// First, let's find overload choice associated with each
// re-labeling fix.
for (const auto &fix : commonFixes) {
auto &solution = *fix.first;

auto calleeLocator = solution.getCalleeLocator(getLocator());
if (!calleeLocator)
return false;

auto overloadChoice = solution.getOverloadChoiceIfAvailable(calleeLocator);
if (!overloadChoice)
return false;

auto *decl = overloadChoice->choice.getDeclOrNull();
if (!decl)
return false;

(void)overloadChoices.insert(decl);
}

// If all of the fixes point to the same overload choice then it's
// exactly the same issue since the call site is static.
if (overloadChoices.size() == 1)
return diagnose(*commonFixes.front().first);

return false;
}

RelabelArguments *
RelabelArguments::create(ConstraintSystem &cs,
llvm::ArrayRef<Identifier> correctLabels,
Expand Down
43 changes: 32 additions & 11 deletions lib/Sema/CSStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -942,13 +942,10 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
// and scoring information.
Snapshot.reset();

// Restore original scores of outer context before
// trying to produce a combined solution.
restoreOriginalScores();

// Apply all of the information deduced from the
// conjunction (up to the point of ambiguity)
// back to the outer context and form a joined solution.
unsigned numSolutions = 0;
for (auto &solution : Solutions) {
ConstraintSystem::SolverScope scope(CS);

Expand All @@ -958,24 +955,47 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
// of the constraint system, so they have to be
// restored right afterwards because score of the
// element does contribute to the overall score.
restoreOriginalScores();
restoreBestScore();
restoreCurrentScore(solution.getFixedScore());

// Transform all of the unbound outer variables into
// placeholders since we are not going to solve for
// each ambguous solution.
{
unsigned numHoles = 0;
for (auto *typeVar : CS.getTypeVariables()) {
if (!typeVar->getImpl().hasRepresentativeOrFixed()) {
CS.assignFixedType(
typeVar, PlaceholderType::get(CS.getASTContext(), typeVar));
++numHoles;
}
}
CS.increaseScore(SK_Hole, numHoles);
}

if (CS.worseThanBestSolution())
continue;

// Note that `worseThanBestSolution` isn't checked
// here because `Solutions` were pre-filtered, and
// outer score is the same for all of them.
OuterSolutions.push_back(CS.finalize());
++numSolutions;
}

return done(/*isSuccess=*/true);
return done(/*isSuccess=*/numSolutions > 0);
}

auto solution = Solutions.pop_back_val();
auto score = solution.getFixedScore();

// Restore outer type variables and prepare to solve
// constraints associated with outer context together
// with information deduced from the conjunction.
Snapshot->setupOuterContext(Solutions.pop_back_val());
Snapshot->setupOuterContext(std::move(solution));

// Pretend that conjunction never happend.
restoreOuterState();
// Pretend that conjunction never happened.
restoreOuterState(score);

// Now that all of the information from the conjunction has
// been applied, let's attempt to solve the outer scope.
Expand All @@ -987,10 +1007,11 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
return take(prevFailed);
}

void ConjunctionStep::restoreOuterState() const {
void ConjunctionStep::restoreOuterState(const Score &solutionScore) const {
// Restore best/current score, since upcoming step is going to
// work with outer scope in relation to the conjunction.
restoreOriginalScores();
restoreBestScore();
restoreCurrentScore(solutionScore);

// Active all of the previously out-of-scope constraints
// because conjunction can propagate type information up
Expand Down
16 changes: 11 additions & 5 deletions lib/Sema/CSStep.h
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,11 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {

// Restore best score only if conjunction fails because
// successful outcome should keep a score set by `restoreOuterState`.
if (HadFailure)
restoreOriginalScores();
if (HadFailure) {
auto solutionScore = Score();
restoreBestScore();
restoreCurrentScore(solutionScore);
}

if (OuterTimeRemaining) {
auto anchor = OuterTimeRemaining->first;
Expand Down Expand Up @@ -1015,16 +1018,19 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {

private:
/// Restore best and current scores as they were before conjunction.
void restoreOriginalScores() const {
CS.solverState->BestScore = BestScore;
void restoreCurrentScore(const Score &solutionScore) const {
CS.CurrentScore = CurrentScore;
CS.increaseScore(SK_Fix, solutionScore.Data[SK_Fix]);
CS.increaseScore(SK_Hole, solutionScore.Data[SK_Hole]);
}

void restoreBestScore() const { CS.solverState->BestScore = BestScore; }

// Restore constraint system state before conjunction.
//
// Note that this doesn't include conjunction constraint
// itself because we don't want to re-solve it.
void restoreOuterState() const;
void restoreOuterState(const Score &solutionScore) const;
};

} // end namespace constraints
Expand Down
27 changes: 22 additions & 5 deletions lib/Serialization/ModuleFileSharedCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,14 @@ static bool readOptionsBlock(llvm::BitstreamCursor &cursor,
static ValidationInfo validateControlBlock(
llvm::BitstreamCursor &cursor, SmallVectorImpl<uint64_t> &scratch,
std::pair<uint16_t, uint16_t> expectedVersion, bool requiresOSSAModules,
bool requiresRevisionMatch,
ExtendedValidationInfo *extendedInfo,
PathObfuscator &pathRecoverer) {
// The control block is malformed until we've at least read a major version
// number.
ValidationInfo result;
bool versionSeen = false;
bool revisionSeen = false;

while (!cursor.AtEndOfStream()) {
Expected<llvm::BitstreamEntry> maybeEntry = cursor.advance();
Expand Down Expand Up @@ -308,6 +310,8 @@ static ValidationInfo validateControlBlock(
break;
}
case control_block::REVISION: {
revisionSeen = true;

// Tagged compilers should only load modules if they were
// produced by the exact same compiler tag.

Expand All @@ -331,8 +335,12 @@ static ValidationInfo validateControlBlock(
if (isCompilerTagged) {
StringRef compilerRevision = forcedDebugRevision ?
forcedDebugRevision : version::getSwiftRevision();
if (moduleRevision != compilerRevision)
if (moduleRevision != compilerRevision) {
result.status = Status::RevisionIncompatible;

// We can't trust this module format at this point.
return result;
}
}
break;
}
Expand All @@ -349,6 +357,13 @@ static ValidationInfo validateControlBlock(
}
}

// Last resort check in cases where the format is broken enough that
// we didn't read the REVISION block, report such a case as incompatible.
if (requiresRevisionMatch &&
!revisionSeen &&
result.status == Status::Valid)
result.status = Status::RevisionIncompatible;

return result;
}

Expand Down Expand Up @@ -471,7 +486,8 @@ ValidationInfo serialization::validateSerializedAST(
result = validateControlBlock(
cursor, scratch,
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
requiresOSSAModules, extendedInfo, localObfuscator);
requiresOSSAModules, /*requiresRevisionMatch=*/true,
extendedInfo, localObfuscator);
if (result.status == Status::Malformed)
return result;
} else if (dependencies &&
Expand Down Expand Up @@ -979,7 +995,7 @@ bool ModuleFileSharedCore::readModuleDocIfPresent(PathObfuscator &pathRecoverer)

info = validateControlBlock(
docCursor, scratch, {SWIFTDOC_VERSION_MAJOR, SWIFTDOC_VERSION_MINOR},
RequiresOSSAModules,
RequiresOSSAModules, /*requiresRevisionMatch=*/false,
/*extendedInfo*/ nullptr, pathRecoverer);
if (info.status != Status::Valid)
return false;
Expand Down Expand Up @@ -1123,7 +1139,7 @@ bool ModuleFileSharedCore::readModuleSourceInfoIfPresent(PathObfuscator &pathRec
info = validateControlBlock(
infoCursor, scratch,
{SWIFTSOURCEINFO_VERSION_MAJOR, SWIFTSOURCEINFO_VERSION_MINOR},
RequiresOSSAModules,
RequiresOSSAModules, /*requiresRevisionMatch=*/false,
/*extendedInfo*/ nullptr,
pathRecoverer);
if (info.status != Status::Valid)
Expand Down Expand Up @@ -1251,7 +1267,8 @@ ModuleFileSharedCore::ModuleFileSharedCore(
info = validateControlBlock(
cursor, scratch,
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
RequiresOSSAModules, &extInfo, pathRecoverer);
RequiresOSSAModules, /*requiresRevisionMatch=*/true,
&extInfo, pathRecoverer);
if (info.status != Status::Valid) {
error(info.status);
return;
Expand Down
1 change: 0 additions & 1 deletion test/Constraints/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,6 @@ func test(arr: [[Int]]) {
}

arr.map { ($0 as? [Int]).map { A($0) } } // expected-error {{missing argument label 'arg:' in call}} {{36-36=arg: }}
// expected-warning@-1 {{conditional cast from '[Int]' to '[Int]' always succeeds}}
}

func closureWithCaseArchetype<T>(_: T.Type) {
Expand Down
3 changes: 2 additions & 1 deletion test/Constraints/generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,8 @@ func rdar78781552() {
// expected-error@-1 {{generic struct 'Test' requires that '(((Int) throws -> Bool) throws -> [Int])?' conform to 'RandomAccessCollection'}}
// expected-error@-2 {{generic parameter 'Content' could not be inferred}} expected-note@-2 {{explicitly specify the generic arguments to fix this issue}}
// expected-error@-3 {{cannot convert value of type '(((Int) throws -> Bool) throws -> [Int])?' to expected argument type '[(((Int) throws -> Bool) throws -> [Int])?]'}}
// expected-error@-4 {{missing argument for parameter 'filter' in call}}
// expected-error@-4 {{missing argument label 'data:' in call}}
// expected-error@-5 {{missing argument for parameter 'filter' in call}}
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/Index/Store/unit-pcm-dependency-remapped.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func test() {
// FILE1-NOT: Unit |{{.*}}ClangModuleA
// FILE1: Record | user | {{.*}}unit-pcm-dependency-remapped.swift | unit-pcm-dependency-remapped.swift-
// FILE1-NOT: Unit |{{.*}}ClangModuleA
// FILE1: DEPEND END (4)
// FILE1: DEPEND END

// FILE2-NOT: main.swiftmodule-

Expand Down
Loading