Skip to content

Commit cd74150

Browse files
authored
Merge pull request #18700 from DougGregor/redundant-duplicate-inheritance
2 parents 8f41ee7 + 36dc43a commit cd74150

File tree

3 files changed

+56
-50
lines changed

3 files changed

+56
-50
lines changed

lib/AST/DeclContext.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,8 @@ ProtocolDecl *DeclContext::getAsProtocolExtensionContext() const {
9191
GenericTypeParamType *DeclContext::getProtocolSelfType() const {
9292
assert(getAsProtocolOrProtocolExtensionContext() && "not a protocol");
9393

94-
auto genericParams = getGenericParamsOfContext();
95-
96-
if (!genericParams) {
97-
if (auto proto = dyn_cast<ProtocolDecl>(this)) {
98-
getASTContext().getLazyResolver()
99-
->resolveDeclSignature(const_cast<ProtocolDecl *>(proto));
100-
genericParams = getGenericParamsOfContext();
101-
}
94+
if (auto proto = dyn_cast<ProtocolDecl>(this)) {
95+
const_cast<ProtocolDecl *>(proto)->createGenericParamsIfMissing();
10296
}
10397

10498
return getGenericParamsOfContext()->getParams().front()

lib/Sema/TypeCheckDecl.cpp

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
352352
// Check all of the types listed in the inheritance clause.
353353
Type superclassTy;
354354
SourceRange superclassRange;
355-
llvm::SmallDenseMap<CanType, std::pair<unsigned, SourceRange>> inheritedTypes;
355+
Optional<std::pair<unsigned, SourceRange>> inheritedAnyObject;
356356
for (unsigned i = 0, n = inheritedClause.size(); i != n; ++i) {
357357
auto &inherited = inheritedClause[i];
358358

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

371-
// Check whether we inherited from the same type twice.
372-
auto knownPair = inheritedTypes.insert({ inheritedTy->getCanonicalType(),
373-
{i, inherited.getSourceRange() }});
374-
if (knownPair.second == /*insertion failed*/false) {
375-
auto knownIndex = knownPair.first->second.first;
376-
auto knownRange = knownPair.first->second.second;
377-
// If the duplicated type is 'AnyObject', check whether the first was
378-
// written as 'class'. Downgrade the error to a warning in such cases
379-
// for backward compatibility with Swift <= 4.
380-
if (!Context.LangOpts.isSwiftVersionAtLeast(5) &&
381-
inheritedTy->isAnyObject() &&
382-
(isa<ProtocolDecl>(decl) || isa<AbstractTypeParamDecl>(decl)) &&
383-
Lexer::getTokenAtLocation(Context.SourceMgr, knownRange.Start)
384-
.is(tok::kw_class)) {
385-
SourceLoc classLoc = knownRange.Start;
371+
// Check whether we inherited from 'AnyObject' twice.
372+
// Other redundant-inheritance scenarios are checked below, the
373+
// GenericSignatureBuilder (for protocol inheritance) or the
374+
// ConformanceLookupTable (for protocol conformance).
375+
if (inheritedTy->isAnyObject()) {
376+
if (inheritedAnyObject) {
377+
// If the first occurrence was written as 'class', downgrade the error
378+
// to a warning in such cases
379+
// for backward compatibility with Swift <= 4.
380+
auto knownIndex = inheritedAnyObject->first;
381+
auto knownRange = inheritedAnyObject->second;
386382
SourceRange removeRange = getRemovalRange(knownIndex);
387-
388-
diagnose(classLoc, diag::duplicate_anyobject_class_inheritance)
389-
.fixItRemoveChars(removeRange.Start, removeRange.End);
390-
inherited.setInvalidType(Context);
383+
if (!Context.LangOpts.isSwiftVersionAtLeast(5) &&
384+
(isa<ProtocolDecl>(decl) || isa<AbstractTypeParamDecl>(decl)) &&
385+
Lexer::getTokenAtLocation(Context.SourceMgr, knownRange.Start)
386+
.is(tok::kw_class)) {
387+
SourceLoc classLoc = knownRange.Start;
388+
389+
diagnose(classLoc, diag::duplicate_anyobject_class_inheritance)
390+
.fixItRemoveChars(removeRange.Start, removeRange.End);
391+
} else {
392+
diagnose(inherited.getSourceRange().Start,
393+
diag::duplicate_inheritance, inheritedTy)
394+
.fixItRemoveChars(removeRange.Start, removeRange.End);
395+
}
391396
continue;
392397
}
393398

394-
auto removeRange = getRemovalRange(i);
395-
diagnose(inherited.getSourceRange().Start,
396-
diag::duplicate_inheritance, inheritedTy)
397-
.fixItRemoveChars(removeRange.Start, removeRange.End)
398-
.highlight(knownRange);
399-
inherited.setInvalidType(Context);
400-
continue;
399+
// Note that we saw inheritance from 'AnyObject'.
400+
inheritedAnyObject = { i, inherited.getSourceRange() };
401401
}
402402

403403
if (inheritedTy->isExistentialType()) {
@@ -455,10 +455,16 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
455455
if (isa<EnumDecl>(decl)) {
456456
// Check if we already had a raw type.
457457
if (superclassTy) {
458-
diagnose(inherited.getSourceRange().Start,
459-
diag::multiple_enum_raw_types, superclassTy, inheritedTy)
460-
.highlight(superclassRange);
461-
inherited.setInvalidType(Context);
458+
if (superclassTy->isEqual(inheritedTy)) {
459+
auto removeRange = getRemovalRange(i);
460+
diagnose(inherited.getSourceRange().Start,
461+
diag::duplicate_inheritance, inheritedTy)
462+
.fixItRemoveChars(removeRange.Start, removeRange.End);
463+
} else {
464+
diagnose(inherited.getSourceRange().Start,
465+
diag::multiple_enum_raw_types, superclassTy, inheritedTy)
466+
.highlight(superclassRange);
467+
}
462468
continue;
463469
}
464470

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

490-
// Complain about multiple inheritance.
491-
// Don't emit a Fix-It here. The user has to think harder about this.
492-
diagnose(inherited.getSourceRange().Start,
493-
diag::multiple_inheritance, superclassTy, inheritedTy)
494-
.highlight(superclassRange);
495-
inherited.setInvalidType(Context);
496+
if (superclassTy->isEqual(inheritedTy)) {
497+
// Duplicate superclass.
498+
auto removeRange = getRemovalRange(i);
499+
diagnose(inherited.getSourceRange().Start,
500+
diag::duplicate_inheritance, inheritedTy)
501+
.fixItRemoveChars(removeRange.Start, removeRange.End);
502+
} else {
503+
// Complain about multiple inheritance.
504+
// Don't emit a Fix-It here. The user has to think harder about this.
505+
diagnose(inherited.getSourceRange().Start,
506+
diag::multiple_inheritance, superclassTy, inheritedTy)
507+
.highlight(superclassRange);
508+
}
496509
continue;
497510
}
498511

test/decl/inherit/inherit.swift

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@ public protocol P1 { }
88
class B : A, A { } // expected-error{{duplicate inheritance from 'A'}}{{12-15=}}
99

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

1615
// Multiple inheritance
1716
class C : B, A { } // expected-error{{multiple inheritance from classes 'B' and 'A'}}
@@ -46,7 +45,7 @@ struct S2 : struct { } // expected-error{{expected type}}
4645
struct S3 : P, P & Q { } // expected-error {{redundant conformance of 'S3' to protocol 'P'}}
4746
// expected-error @-1 {{non-class type 'S3' cannot conform to class protocol 'Q'}}
4847
// expected-note @-2 {{'S3' declares conformance to protocol 'P' here}}
49-
struct S4 : P, P { } // FIXME: expected-error {{duplicate inheritance from 'P'}}
48+
struct S4 : P, P { }
5049
// expected-error@-1{{redundant conformance of 'S4' to protocol 'P'}}
5150
// expected-note@-2{{'S4' declares conformance to protocol 'P' here}}
5251
struct S6 : P & { } // expected-error {{expected identifier for type name}}

0 commit comments

Comments
 (0)