Skip to content

[Sema] Shepherd @manavgabhawala's changes to circular protocol diagnostics #3747

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.

Sign up for GitHub

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
Jul 26, 2016
Merged

[Sema] Shepherd @manavgabhawala's changes to circular protocol diagnostics #3747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d06d4e5
[AST][Sema] Fixes the IterativeTypeChecker and better manages circula…
manavgabhawala Apr 10, 2016
f8fa87a
Revert IterativeTypeChecker changes
CodaFi Jul 25, 2016
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
6 changes: 4 additions & 2 deletions lib/AST/ConformanceLookupTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ bool ConformanceLookupTable::addProtocol(NominalTypeDecl *nominal,

// If this entry is synthesized or implied, scan to determine
// whether there are any explicit better conformances that make this
// conformance trivially superseded (and, therefore, no worth
// conformance trivially superseded (and, therefore, not worth
// recording).
auto &conformanceEntries = Conformances[protocol];
if (kind == ConformanceEntryKind::Implied ||
Expand All @@ -458,7 +458,9 @@ bool ConformanceLookupTable::addProtocol(NominalTypeDecl *nominal,

case ConformanceEntryKind::Implied:
// An implied conformance is better than a synthesized one.
if (kind == ConformanceEntryKind::Synthesized)
// Ignore implied circular protocol inheritance
if (kind == ConformanceEntryKind::Synthesized ||
existingEntry->getProtocol() == protocol)
return false;
break;

Expand Down
40 changes: 15 additions & 25 deletions lib/Sema/ITCDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ void IterativeTypeChecker::processInheritedProtocols(
// FIXME: Technically, we only need very basic name binding.
auto inheritedClause = protocol->getInherited();
bool anyDependencies = false;
bool diagnosedCircularity = false;
llvm::SmallSetVector<ProtocolDecl *, 4> allProtocols;
for (unsigned i = 0, n = inheritedClause.size(); i != n; ++i) {
TypeLoc &inherited = inheritedClause[i];
Expand All @@ -237,43 +238,32 @@ void IterativeTypeChecker::processInheritedProtocols(
// FIXME: We'd prefer to keep what the user wrote here.
SmallVector<ProtocolDecl *, 4> protocols;
if (inherited.getType()->isExistentialType(protocols)) {
allProtocols.insert(protocols.begin(), protocols.end());
continue;
for (auto inheritedProtocol: protocols) {
if (inheritedProtocol == protocol ||
inheritedProtocol->inheritsFrom(protocol)) {
if (!diagnosedCircularity &&
!protocol->isInheritedProtocolsValid()) {
diagnose(protocol,
diag::circular_protocol_def, protocol->getName().str())
.fixItRemove(inherited.getSourceRange());
diagnosedCircularity = true;
}
continue;
}
allProtocols.insert(inheritedProtocol);
}
}
}

// If we enumerated any dependencies, we can't complete this request.
if (anyDependencies)
return;

// Check for circular inheritance.
// FIXME: The diagnostics here should be improved... and this should probably
// be handled by the normal cycle detection.
bool diagnosedCircularity = false;
for (unsigned i = 0, n = allProtocols.size(); i != n; /*in loop*/) {
if (allProtocols[i] == protocol ||
allProtocols[i]->inheritsFrom(protocol)) {
if (!diagnosedCircularity &&
!protocol->isInheritedProtocolsValid()) {
diagnose(protocol,
diag::circular_protocol_def, protocol->getName().str());
diagnosedCircularity = true;
}

allProtocols.remove(allProtocols[i]);
--n;
continue;
}

++i;
}

// FIXME: Hack to deal with recursion elsewhere.
// We recurse through DeclContext::getLocalProtocols() -- this should be
// redone to use the IterativeDeclChecker also.
if (protocol->isInheritedProtocolsValid())
return;

protocol->setInheritedProtocols(getASTContext().AllocateCopy(allProtocols));
}

Expand Down
2 changes: 1 addition & 1 deletion ...icconsumer-handlediagnostic.timeout.swift → ...icconsumer-handlediagnostic.timeout.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// See http://swift.org/LICENSE.txt for license information
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors

// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// REQUIRES: asserts
protocol c:A
protocol A:d
Expand Down