Skip to content

Commit f4359b7

Browse files
authored
Merge pull request #17729 from DougGregor/minimize-override-checking
[Type checker] Minimize checking needed to compute the set of overridden declarations
2 parents 47f13ec + 9939834 commit f4359b7

File tree

10 files changed

+222
-115
lines changed

10 files changed

+222
-115
lines changed

lib/Sema/CodeSynthesis.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2412,6 +2412,10 @@ static void configureDesignatedInitAttributes(TypeChecker &tc,
24122412
ctor, {classDecl, superclassCtor}, ctx);
24132413
}
24142414

2415+
// Wire up the overrides.
2416+
ctor->getAttrs().add(new (ctx) OverrideAttr(/*IsImplicit=*/true));
2417+
ctor->setOverriddenDecl(superclassCtor);
2418+
24152419
if (superclassCtor->isObjC()) {
24162420
// Inherit the @objc name from the superclass initializer, if it
24172421
// has one.
@@ -2430,10 +2434,6 @@ static void configureDesignatedInitAttributes(TypeChecker &tc,
24302434
ctor->getAttrs().add(new (ctx) RequiredAttr(/*IsImplicit=*/true));
24312435
if (superclassCtor->isDynamic())
24322436
ctor->getAttrs().add(new (ctx) DynamicAttr(/*IsImplicit*/true));
2433-
2434-
// Wire up the overrides.
2435-
ctor->getAttrs().add(new (ctx) OverrideAttr(/*IsImplicit=*/true));
2436-
ctor->setOverriddenDecl(superclassCtor);
24372437
}
24382438

24392439
ConstructorDecl *

lib/Sema/TypeCheckDecl.cpp

Lines changed: 98 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,12 +1393,17 @@ static void inferDynamic(ASTContext &ctx, ValueDecl *D) {
13931393
(D->getOverriddenDecl() &&
13941394
D->getOverriddenDecl()->hasClangNode());
13951395

1396+
bool overridesDyanmic =
1397+
(D->getOverriddenDecl() &&
1398+
D->getOverriddenDecl()->isDynamic());
1399+
13961400
bool isNSManaged = D->getAttrs().hasAttribute<NSManagedAttr>();
13971401

13981402
bool isExtension = isa<ExtensionDecl>(D->getDeclContext());
13991403

14001404
// We only infer 'dynamic' in these three cases.
1401-
if (!isExtension && !isNSManaged && !overridesImportedMethod)
1405+
if (!isExtension && !isNSManaged && !overridesImportedMethod &&
1406+
!overridesDyanmic)
14021407
return;
14031408

14041409
// The presence of 'final' blocks the inference of 'dynamic'.
@@ -2904,6 +2909,19 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
29042909
}
29052910
}
29062911

2912+
if (!checkOverrides(TC, VD)) {
2913+
// If a property has an override attribute but does not override
2914+
// anything, complain.
2915+
auto overridden = VD->getOverriddenDecl();
2916+
if (auto *OA = VD->getAttrs().getAttribute<OverrideAttr>()) {
2917+
if (!overridden) {
2918+
TC.diagnose(VD, diag::property_does_not_override)
2919+
.highlight(OA->getLocation());
2920+
OA->setInvalid();
2921+
}
2922+
}
2923+
}
2924+
29072925
TC.checkDeclAttributes(VD);
29082926

29092927
triggerAccessorSynthesis(TC, VD);
@@ -3049,6 +3067,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
30493067
AccessControlChecker::checkAccessControl(TC, SD);
30503068
UsableFromInlineChecker::checkUsableFromInline(TC, SD);
30513069

3070+
if (!checkOverrides(TC, SD)) {
3071+
// If a subscript has an override attribute but does not override
3072+
// anything, complain.
3073+
if (auto *OA = SD->getAttrs().getAttribute<OverrideAttr>()) {
3074+
if (!SD->getOverriddenDecl()) {
3075+
TC.diagnose(SD, diag::subscript_does_not_override)
3076+
.highlight(OA->getLocation());
3077+
OA->setInvalid();
3078+
}
3079+
}
3080+
}
3081+
30523082
triggerAccessorSynthesis(TC, SD);
30533083
}
30543084

@@ -3539,6 +3569,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
35393569
AccessControlChecker::checkAccessControl(TC, FD);
35403570
UsableFromInlineChecker::checkUsableFromInline(TC, FD);
35413571

3572+
if (!checkOverrides(TC, FD)) {
3573+
// If a method has an 'override' keyword but does not
3574+
// override anything, complain.
3575+
if (auto *OA = FD->getAttrs().getAttribute<OverrideAttr>()) {
3576+
if (!FD->getOverriddenDecl()) {
3577+
TC.diagnose(FD, diag::method_does_not_override)
3578+
.highlight(OA->getLocation());
3579+
OA->setInvalid();
3580+
}
3581+
}
3582+
}
3583+
35423584
if (FD->hasBody()) {
35433585
// Record the body.
35443586
TC.definedFunctions.push_back(FD);
@@ -3668,6 +3710,61 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
36683710
void visitConstructorDecl(ConstructorDecl *CD) {
36693711
TC.validateDecl(CD);
36703712

3713+
// Check whether this initializer overrides an initializer in its
3714+
// superclass.
3715+
if (!checkOverrides(TC, CD)) {
3716+
// If an initializer has an override attribute but does not override
3717+
// anything or overrides something that doesn't need an 'override'
3718+
// keyword (e.g., a convenience initializer), complain.
3719+
// anything, or overrides something that complain.
3720+
if (auto *attr = CD->getAttrs().getAttribute<OverrideAttr>()) {
3721+
if (!CD->getOverriddenDecl()) {
3722+
TC.diagnose(CD, diag::initializer_does_not_override)
3723+
.highlight(attr->getLocation());
3724+
attr->setInvalid();
3725+
} else if (attr->isImplicit()) {
3726+
// Don't diagnose implicit attributes.
3727+
} else if (!overrideRequiresKeyword(CD->getOverriddenDecl())) {
3728+
// Special case: we are overriding a 'required' initializer, so we
3729+
// need (only) the 'required' keyword.
3730+
if (cast<ConstructorDecl>(CD->getOverriddenDecl())->isRequired()) {
3731+
if (CD->getAttrs().hasAttribute<RequiredAttr>()) {
3732+
TC.diagnose(CD, diag::required_initializer_override_keyword)
3733+
.fixItRemove(attr->getLocation());
3734+
} else {
3735+
TC.diagnose(CD, diag::required_initializer_override_wrong_keyword)
3736+
.fixItReplace(attr->getLocation(), "required");
3737+
CD->getAttrs().add(
3738+
new (TC.Context) RequiredAttr(/*IsImplicit=*/true));
3739+
}
3740+
3741+
TC.diagnose(findNonImplicitRequiredInit(CD->getOverriddenDecl()),
3742+
diag::overridden_required_initializer_here);
3743+
} else {
3744+
// We tried to override a convenience initializer.
3745+
TC.diagnose(CD, diag::initializer_does_not_override)
3746+
.highlight(attr->getLocation());
3747+
TC.diagnose(CD->getOverriddenDecl(),
3748+
diag::convenience_init_override_here);
3749+
}
3750+
}
3751+
}
3752+
3753+
// A failable initializer cannot override a non-failable one.
3754+
// This would normally be diagnosed by the covariance rules;
3755+
// however, those are disabled so that we can provide a more
3756+
// specific diagnostic here.
3757+
if (CD->getFailability() != OTK_None &&
3758+
CD->getOverriddenDecl() &&
3759+
CD->getOverriddenDecl()->getFailability() == OTK_None) {
3760+
TC.diagnose(CD, diag::failable_initializer_override,
3761+
CD->getFullName());
3762+
TC.diagnose(CD->getOverriddenDecl(),
3763+
diag::nonfailable_initializer_override_here,
3764+
CD->getOverriddenDecl()->getFullName());
3765+
}
3766+
}
3767+
36713768
// If this initializer overrides a 'required' initializer, it must itself
36723769
// be marked 'required'.
36733770
if (!CD->getAttrs().hasAttribute<RequiredAttr>()) {
@@ -4440,19 +4537,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
44404537
checkDeclAttributesEarly(VD);
44414538
validateAttributes(*this, VD);
44424539

4443-
if (!checkOverrides(*this, VD)) {
4444-
// If a property has an override attribute but does not override
4445-
// anything, complain.
4446-
auto overridden = VD->getOverriddenDecl();
4447-
if (auto *OA = VD->getAttrs().getAttribute<OverrideAttr>()) {
4448-
if (!overridden) {
4449-
diagnose(VD, diag::property_does_not_override)
4450-
.highlight(OA->getLocation());
4451-
OA->setInvalid();
4452-
}
4453-
}
4454-
}
4455-
44564540
// Properties need some special validation logic.
44574541
if (auto *nominalDecl = VD->getDeclContext()
44584542
->getAsNominalTypeOrNominalTypeExtensionContext()) {
@@ -4731,18 +4815,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
47314815

47324816
// Member functions need some special validation logic.
47334817
if (FD->getDeclContext()->isTypeContext()) {
4734-
if (!checkOverrides(*this, FD)) {
4735-
// If a method has an 'override' keyword but does not
4736-
// override anything, complain.
4737-
if (auto *OA = FD->getAttrs().getAttribute<OverrideAttr>()) {
4738-
if (!FD->getOverriddenDecl()) {
4739-
diagnose(FD, diag::method_does_not_override)
4740-
.highlight(OA->getLocation());
4741-
OA->setInvalid();
4742-
}
4743-
}
4744-
}
4745-
47464818
if (FD->isOperator())
47474819
checkMemberOperator(*this, FD);
47484820

@@ -4929,59 +5001,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
49295001

49305002
validateAttributes(*this, CD);
49315003

4932-
// Check whether this initializer overrides an initializer in its
4933-
// superclass.
4934-
if (!checkOverrides(*this, CD)) {
4935-
// If an initializer has an override attribute but does not override
4936-
// anything or overrides something that doesn't need an 'override'
4937-
// keyword (e.g., a convenience initializer), complain.
4938-
// anything, or overrides something that complain.
4939-
if (auto *attr = CD->getAttrs().getAttribute<OverrideAttr>()) {
4940-
if (!CD->getOverriddenDecl()) {
4941-
diagnose(CD, diag::initializer_does_not_override)
4942-
.highlight(attr->getLocation());
4943-
attr->setInvalid();
4944-
} else if (!overrideRequiresKeyword(CD->getOverriddenDecl())) {
4945-
// Special case: we are overriding a 'required' initializer, so we
4946-
// need (only) the 'required' keyword.
4947-
if (cast<ConstructorDecl>(CD->getOverriddenDecl())->isRequired()) {
4948-
if (CD->getAttrs().hasAttribute<RequiredAttr>()) {
4949-
diagnose(CD, diag::required_initializer_override_keyword)
4950-
.fixItRemove(attr->getLocation());
4951-
} else {
4952-
diagnose(CD, diag::required_initializer_override_wrong_keyword)
4953-
.fixItReplace(attr->getLocation(), "required");
4954-
CD->getAttrs().add(
4955-
new (Context) RequiredAttr(/*IsImplicit=*/true));
4956-
}
4957-
4958-
diagnose(findNonImplicitRequiredInit(CD->getOverriddenDecl()),
4959-
diag::overridden_required_initializer_here);
4960-
} else {
4961-
// We tried to override a convenience initializer.
4962-
diagnose(CD, diag::initializer_does_not_override)
4963-
.highlight(attr->getLocation());
4964-
diagnose(CD->getOverriddenDecl(),
4965-
diag::convenience_init_override_here);
4966-
}
4967-
}
4968-
}
4969-
4970-
// A failable initializer cannot override a non-failable one.
4971-
// This would normally be diagnosed by the covariance rules;
4972-
// however, those are disabled so that we can provide a more
4973-
// specific diagnostic here.
4974-
if (CD->getFailability() != OTK_None &&
4975-
CD->getOverriddenDecl() &&
4976-
CD->getOverriddenDecl()->getFailability() == OTK_None) {
4977-
diagnose(CD, diag::failable_initializer_override,
4978-
CD->getFullName());
4979-
diagnose(CD->getOverriddenDecl(),
4980-
diag::nonfailable_initializer_override_here,
4981-
CD->getOverriddenDecl()->getFullName());
4982-
}
4983-
}
4984-
49855004
// An initializer is ObjC-compatible if it's explicitly @objc or a member
49865005
// of an ObjC-compatible class.
49875006
if (CD->getDeclContext()->isTypeContext()) {
@@ -5123,18 +5142,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
51235142
new (C) ImplicitlyUnwrappedOptionalAttr(/* implicit= */ true));
51245143
}
51255144

5126-
if (!checkOverrides(*this, SD)) {
5127-
// If a subscript has an override attribute but does not override
5128-
// anything, complain.
5129-
if (auto *OA = SD->getAttrs().getAttribute<OverrideAttr>()) {
5130-
if (!SD->getOverriddenDecl()) {
5131-
diagnose(SD, diag::subscript_does_not_override)
5132-
.highlight(OA->getLocation());
5133-
OA->setInvalid();
5134-
}
5135-
}
5136-
}
5137-
51385145
// Member subscripts need some special validation logic.
51395146
if (dc->isTypeContext()) {
51405147
// If this is a class member, mark it final if the class is final.

0 commit comments

Comments
 (0)