Skip to content

[Type checker] Minimize checking needed to compute the set of overridden declarations #17729

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 3 commits into from
Jul 5, 2018
Merged

[Type checker] Minimize checking needed to compute the set of overridden declarations #17729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7279eee
[Type checker] Perform more minimal checking in resolveOverriddenDecl().
DougGregor Jul 4, 2018
a3e03fa
[Type checker] Make sure 'dynamic' gets set consistently.
DougGregor Jul 4, 2018
9939834
[Type checker] Validate potentially-overridden decls when needed.
DougGregor Jul 5, 2018
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
8 changes: 4 additions & 4 deletions lib/Sema/CodeSynthesis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2411,6 +2411,10 @@ static void configureDesignatedInitAttributes(TypeChecker &tc,
ctor, {classDecl, superclassCtor}, ctx);
}

// Wire up the overrides.
ctor->getAttrs().add(new (ctx) OverrideAttr(/*IsImplicit=*/true));
ctor->setOverriddenDecl(superclassCtor);

if (superclassCtor->isObjC()) {
// Inherit the @objc name from the superclass initializer, if it
// has one.
Expand All @@ -2429,10 +2433,6 @@ static void configureDesignatedInitAttributes(TypeChecker &tc,
ctor->getAttrs().add(new (ctx) RequiredAttr(/*IsImplicit=*/true));
if (superclassCtor->isDynamic())
ctor->getAttrs().add(new (ctx) DynamicAttr(/*IsImplicit*/true));

// Wire up the overrides.
ctor->getAttrs().add(new (ctx) OverrideAttr(/*IsImplicit=*/true));
ctor->setOverriddenDecl(superclassCtor);
}

ConstructorDecl *
Expand Down
189 changes: 98 additions & 91 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1454,12 +1454,17 @@ static void inferDynamic(ASTContext &ctx, ValueDecl *D) {
(D->getOverriddenDecl() &&
D->getOverriddenDecl()->hasClangNode());

bool overridesDyanmic =
(D->getOverriddenDecl() &&
D->getOverriddenDecl()->isDynamic());

bool isNSManaged = D->getAttrs().hasAttribute<NSManagedAttr>();

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

// We only infer 'dynamic' in these three cases.
if (!isExtension && !isNSManaged && !overridesImportedMethod)
if (!isExtension && !isNSManaged && !overridesImportedMethod &&
!overridesDyanmic)
return;

// The presence of 'final' blocks the inference of 'dynamic'.
Expand Down Expand Up @@ -2965,6 +2970,19 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
}
}

if (!checkOverrides(TC, VD)) {
// If a property has an override attribute but does not override
// anything, complain.
auto overridden = VD->getOverriddenDecl();
if (auto *OA = VD->getAttrs().getAttribute<OverrideAttr>()) {
if (!overridden) {
TC.diagnose(VD, diag::property_does_not_override)
.highlight(OA->getLocation());
OA->setInvalid();
}
}
}

TC.checkDeclAttributes(VD);

triggerAccessorSynthesis(TC, VD);
Expand Down Expand Up @@ -3110,6 +3128,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
AccessControlChecker::checkAccessControl(TC, SD);
UsableFromInlineChecker::checkUsableFromInline(TC, SD);

if (!checkOverrides(TC, SD)) {
// If a subscript has an override attribute but does not override
// anything, complain.
if (auto *OA = SD->getAttrs().getAttribute<OverrideAttr>()) {
if (!SD->getOverriddenDecl()) {
TC.diagnose(SD, diag::subscript_does_not_override)
.highlight(OA->getLocation());
OA->setInvalid();
}
}
}

triggerAccessorSynthesis(TC, SD);
}

Expand Down Expand Up @@ -3605,6 +3635,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
AccessControlChecker::checkAccessControl(TC, FD);
UsableFromInlineChecker::checkUsableFromInline(TC, FD);

if (!checkOverrides(TC, FD)) {
// If a method has an 'override' keyword but does not
// override anything, complain.
if (auto *OA = FD->getAttrs().getAttribute<OverrideAttr>()) {
if (!FD->getOverriddenDecl()) {
TC.diagnose(FD, diag::method_does_not_override)
.highlight(OA->getLocation());
OA->setInvalid();
}
}
}

if (FD->hasBody()) {
// Record the body.
TC.definedFunctions.push_back(FD);
Expand Down Expand Up @@ -3734,6 +3776,61 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
void visitConstructorDecl(ConstructorDecl *CD) {
TC.validateDecl(CD);

// Check whether this initializer overrides an initializer in its
// superclass.
if (!checkOverrides(TC, CD)) {
// If an initializer has an override attribute but does not override
// anything or overrides something that doesn't need an 'override'
// keyword (e.g., a convenience initializer), complain.
// anything, or overrides something that complain.
if (auto *attr = CD->getAttrs().getAttribute<OverrideAttr>()) {
if (!CD->getOverriddenDecl()) {
TC.diagnose(CD, diag::initializer_does_not_override)
.highlight(attr->getLocation());
attr->setInvalid();
} else if (attr->isImplicit()) {
// Don't diagnose implicit attributes.
} else if (!overrideRequiresKeyword(CD->getOverriddenDecl())) {
// Special case: we are overriding a 'required' initializer, so we
// need (only) the 'required' keyword.
if (cast<ConstructorDecl>(CD->getOverriddenDecl())->isRequired()) {
if (CD->getAttrs().hasAttribute<RequiredAttr>()) {
TC.diagnose(CD, diag::required_initializer_override_keyword)
.fixItRemove(attr->getLocation());
} else {
TC.diagnose(CD, diag::required_initializer_override_wrong_keyword)
.fixItReplace(attr->getLocation(), "required");
CD->getAttrs().add(
new (TC.Context) RequiredAttr(/*IsImplicit=*/true));
}

TC.diagnose(findNonImplicitRequiredInit(CD->getOverriddenDecl()),
diag::overridden_required_initializer_here);
} else {
// We tried to override a convenience initializer.
TC.diagnose(CD, diag::initializer_does_not_override)
.highlight(attr->getLocation());
TC.diagnose(CD->getOverriddenDecl(),
diag::convenience_init_override_here);
}
}
}

// A failable initializer cannot override a non-failable one.
// This would normally be diagnosed by the covariance rules;
// however, those are disabled so that we can provide a more
// specific diagnostic here.
if (CD->getFailability() != OTK_None &&
CD->getOverriddenDecl() &&
CD->getOverriddenDecl()->getFailability() == OTK_None) {
TC.diagnose(CD, diag::failable_initializer_override,
CD->getFullName());
TC.diagnose(CD->getOverriddenDecl(),
diag::nonfailable_initializer_override_here,
CD->getOverriddenDecl()->getFullName());
}
}

// If this initializer overrides a 'required' initializer, it must itself
// be marked 'required'.
if (!CD->getAttrs().hasAttribute<RequiredAttr>()) {
Expand Down Expand Up @@ -4506,19 +4603,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
checkDeclAttributesEarly(VD);
validateAttributes(*this, VD);

if (!checkOverrides(*this, VD)) {
// If a property has an override attribute but does not override
// anything, complain.
auto overridden = VD->getOverriddenDecl();
if (auto *OA = VD->getAttrs().getAttribute<OverrideAttr>()) {
if (!overridden) {
diagnose(VD, diag::property_does_not_override)
.highlight(OA->getLocation());
OA->setInvalid();
}
}
}

// Properties need some special validation logic.
if (auto *nominalDecl = VD->getDeclContext()
->getAsNominalTypeOrNominalTypeExtensionContext()) {
Expand Down Expand Up @@ -4797,18 +4881,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {

// Member functions need some special validation logic.
if (FD->getDeclContext()->isTypeContext()) {
if (!checkOverrides(*this, FD)) {
// If a method has an 'override' keyword but does not
// override anything, complain.
if (auto *OA = FD->getAttrs().getAttribute<OverrideAttr>()) {
if (!FD->getOverriddenDecl()) {
diagnose(FD, diag::method_does_not_override)
.highlight(OA->getLocation());
OA->setInvalid();
}
}
}

if (FD->isOperator())
checkMemberOperator(*this, FD);

Expand Down Expand Up @@ -4995,59 +5067,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {

validateAttributes(*this, CD);

// Check whether this initializer overrides an initializer in its
// superclass.
if (!checkOverrides(*this, CD)) {
// If an initializer has an override attribute but does not override
// anything or overrides something that doesn't need an 'override'
// keyword (e.g., a convenience initializer), complain.
// anything, or overrides something that complain.
if (auto *attr = CD->getAttrs().getAttribute<OverrideAttr>()) {
if (!CD->getOverriddenDecl()) {
diagnose(CD, diag::initializer_does_not_override)
.highlight(attr->getLocation());
attr->setInvalid();
} else if (!overrideRequiresKeyword(CD->getOverriddenDecl())) {
// Special case: we are overriding a 'required' initializer, so we
// need (only) the 'required' keyword.
if (cast<ConstructorDecl>(CD->getOverriddenDecl())->isRequired()) {
if (CD->getAttrs().hasAttribute<RequiredAttr>()) {
diagnose(CD, diag::required_initializer_override_keyword)
.fixItRemove(attr->getLocation());
} else {
diagnose(CD, diag::required_initializer_override_wrong_keyword)
.fixItReplace(attr->getLocation(), "required");
CD->getAttrs().add(
new (Context) RequiredAttr(/*IsImplicit=*/true));
}

diagnose(findNonImplicitRequiredInit(CD->getOverriddenDecl()),
diag::overridden_required_initializer_here);
} else {
// We tried to override a convenience initializer.
diagnose(CD, diag::initializer_does_not_override)
.highlight(attr->getLocation());
diagnose(CD->getOverriddenDecl(),
diag::convenience_init_override_here);
}
}
}

// A failable initializer cannot override a non-failable one.
// This would normally be diagnosed by the covariance rules;
// however, those are disabled so that we can provide a more
// specific diagnostic here.
if (CD->getFailability() != OTK_None &&
CD->getOverriddenDecl() &&
CD->getOverriddenDecl()->getFailability() == OTK_None) {
diagnose(CD, diag::failable_initializer_override,
CD->getFullName());
diagnose(CD->getOverriddenDecl(),
diag::nonfailable_initializer_override_here,
CD->getOverriddenDecl()->getFullName());
}
}

// An initializer is ObjC-compatible if it's explicitly @objc or a member
// of an ObjC-compatible class.
if (CD->getDeclContext()->isTypeContext()) {
Expand Down Expand Up @@ -5189,18 +5208,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
new (C) ImplicitlyUnwrappedOptionalAttr(/* implicit= */ true));
}

if (!checkOverrides(*this, SD)) {
// If a subscript has an override attribute but does not override
// anything, complain.
if (auto *OA = SD->getAttrs().getAttribute<OverrideAttr>()) {
if (!SD->getOverriddenDecl()) {
diagnose(SD, diag::subscript_does_not_override)
.highlight(OA->getLocation());
OA->setInvalid();
}
}
}

// Member subscripts need some special validation logic.
if (dc->isTypeContext()) {
// If this is a class member, mark it final if the class is final.
Expand Down
Loading