Skip to content

[Type checker] Eliminate redundant "duplicate inheritance" diagnostics. #18700

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
Aug 14, 2018
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
10 changes: 2 additions & 8 deletions lib/AST/DeclContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,8 @@ ProtocolDecl *DeclContext::getAsProtocolExtensionContext() const {
GenericTypeParamType *DeclContext::getProtocolSelfType() const {
assert(getAsProtocolOrProtocolExtensionContext() && "not a protocol");

auto genericParams = getGenericParamsOfContext();

if (!genericParams) {
if (auto proto = dyn_cast<ProtocolDecl>(this)) {
getASTContext().getLazyResolver()
->resolveDeclSignature(const_cast<ProtocolDecl *>(proto));
genericParams = getGenericParamsOfContext();
}
if (auto proto = dyn_cast<ProtocolDecl>(this)) {
const_cast<ProtocolDecl *>(proto)->createGenericParamsIfMissing();
}

return getGenericParamsOfContext()->getParams().front()
Expand Down
87 changes: 50 additions & 37 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
// Check all of the types listed in the inheritance clause.
Type superclassTy;
SourceRange superclassRange;
llvm::SmallDenseMap<CanType, std::pair<unsigned, SourceRange>> inheritedTypes;
Optional<std::pair<unsigned, SourceRange>> inheritedAnyObject;
for (unsigned i = 0, n = inheritedClause.size(); i != n; ++i) {
auto &inherited = inheritedClause[i];

Expand All @@ -368,36 +368,36 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
if (inheritedTy->hasError())
continue;

// Check whether we inherited from the same type twice.
auto knownPair = inheritedTypes.insert({ inheritedTy->getCanonicalType(),
{i, inherited.getSourceRange() }});
if (knownPair.second == /*insertion failed*/false) {
auto knownIndex = knownPair.first->second.first;
auto knownRange = knownPair.first->second.second;
// If the duplicated type is 'AnyObject', check whether the first was
// written as 'class'. Downgrade the error to a warning in such cases
// for backward compatibility with Swift <= 4.
if (!Context.LangOpts.isSwiftVersionAtLeast(5) &&
inheritedTy->isAnyObject() &&
(isa<ProtocolDecl>(decl) || isa<AbstractTypeParamDecl>(decl)) &&
Lexer::getTokenAtLocation(Context.SourceMgr, knownRange.Start)
.is(tok::kw_class)) {
SourceLoc classLoc = knownRange.Start;
// Check whether we inherited from 'AnyObject' twice.
// Other redundant-inheritance scenarios are checked below, the
// GenericSignatureBuilder (for protocol inheritance) or the
// ConformanceLookupTable (for protocol conformance).
if (inheritedTy->isAnyObject()) {
if (inheritedAnyObject) {
// If the first occurrence was written as 'class', downgrade the error
// to a warning in such cases
// for backward compatibility with Swift <= 4.
auto knownIndex = inheritedAnyObject->first;
auto knownRange = inheritedAnyObject->second;
SourceRange removeRange = getRemovalRange(knownIndex);

diagnose(classLoc, diag::duplicate_anyobject_class_inheritance)
.fixItRemoveChars(removeRange.Start, removeRange.End);
inherited.setInvalidType(Context);
if (!Context.LangOpts.isSwiftVersionAtLeast(5) &&
(isa<ProtocolDecl>(decl) || isa<AbstractTypeParamDecl>(decl)) &&
Lexer::getTokenAtLocation(Context.SourceMgr, knownRange.Start)
.is(tok::kw_class)) {
SourceLoc classLoc = knownRange.Start;

diagnose(classLoc, diag::duplicate_anyobject_class_inheritance)
.fixItRemoveChars(removeRange.Start, removeRange.End);
} else {
diagnose(inherited.getSourceRange().Start,
diag::duplicate_inheritance, inheritedTy)
.fixItRemoveChars(removeRange.Start, removeRange.End);
}
continue;
}

auto removeRange = getRemovalRange(i);
diagnose(inherited.getSourceRange().Start,
diag::duplicate_inheritance, inheritedTy)
.fixItRemoveChars(removeRange.Start, removeRange.End)
.highlight(knownRange);
inherited.setInvalidType(Context);
continue;
// Note that we saw inheritance from 'AnyObject'.
inheritedAnyObject = { i, inherited.getSourceRange() };
}

if (inheritedTy->isExistentialType()) {
Expand Down Expand Up @@ -455,10 +455,16 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
if (isa<EnumDecl>(decl)) {
// Check if we already had a raw type.
if (superclassTy) {
diagnose(inherited.getSourceRange().Start,
diag::multiple_enum_raw_types, superclassTy, inheritedTy)
.highlight(superclassRange);
inherited.setInvalidType(Context);
if (superclassTy->isEqual(inheritedTy)) {
auto removeRange = getRemovalRange(i);
diagnose(inherited.getSourceRange().Start,
diag::duplicate_inheritance, inheritedTy)
.fixItRemoveChars(removeRange.Start, removeRange.End);
} else {
diagnose(inherited.getSourceRange().Start,
diag::multiple_enum_raw_types, superclassTy, inheritedTy)
.highlight(superclassRange);
}
continue;
}

Expand Down Expand Up @@ -487,12 +493,19 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
if (superclassTy) {
// FIXME: Check for shadowed protocol names, i.e., NSObject?

// Complain about multiple inheritance.
// Don't emit a Fix-It here. The user has to think harder about this.
diagnose(inherited.getSourceRange().Start,
diag::multiple_inheritance, superclassTy, inheritedTy)
.highlight(superclassRange);
inherited.setInvalidType(Context);
if (superclassTy->isEqual(inheritedTy)) {
// Duplicate superclass.
auto removeRange = getRemovalRange(i);
diagnose(inherited.getSourceRange().Start,
diag::duplicate_inheritance, inheritedTy)
.fixItRemoveChars(removeRange.Start, removeRange.End);
} else {
// Complain about multiple inheritance.
// Don't emit a Fix-It here. The user has to think harder about this.
diagnose(inherited.getSourceRange().Start,
diag::multiple_inheritance, superclassTy, inheritedTy)
.highlight(superclassRange);
}
continue;
}

Expand Down
9 changes: 4 additions & 5 deletions test/decl/inherit/inherit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ public protocol P1 { }
class B : A, A { } // expected-error{{duplicate inheritance from 'A'}}{{12-15=}}

// Duplicate inheritance from protocol
class B2 : P, P { } // expected-error{{duplicate inheritance from 'P'}}{{13-16=}}
// FIXME: These are unnecessary
// expected-note@-2 {{'B2' declares conformance to protocol 'P' here}}
// expected-error@-3 {{redundant conformance of 'B2' to protocol 'P'}}
class B2 : P, P { }
// expected-note@-1 {{'B2' declares conformance to protocol 'P' here}}
// expected-error@-2 {{redundant conformance of 'B2' to protocol 'P'}}

// Multiple inheritance
class C : B, A { } // expected-error{{multiple inheritance from classes 'B' and 'A'}}
Expand Down Expand Up @@ -46,7 +45,7 @@ struct S2 : struct { } // expected-error{{expected type}}
struct S3 : P, P & Q { } // expected-error {{redundant conformance of 'S3' to protocol 'P'}}
// expected-error @-1 {{non-class type 'S3' cannot conform to class protocol 'Q'}}
// expected-note @-2 {{'S3' declares conformance to protocol 'P' here}}
struct S4 : P, P { } // FIXME: expected-error {{duplicate inheritance from 'P'}}
struct S4 : P, P { }
// expected-error@-1{{redundant conformance of 'S4' to protocol 'P'}}
// expected-note@-2{{'S4' declares conformance to protocol 'P' here}}
struct S6 : P & { } // expected-error {{expected identifier for type name}}
Expand Down