Skip to content

Commit e9465a6

Browse files
slavapestovslavapestov
authored
Merge pull request #18376 from slavapestov/no-overrides-in-protocols-5.0
Don't set overrides on imported protocol requirements [5.0]
2 parents 4830e1f + e440e1d commit e9465a6

File tree

3 files changed

+86
-41
lines changed

3 files changed

+86
-41
lines changed

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 62 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,7 @@ static AccessorDecl *makeEnumRawValueGetter(ClangImporter::Implementation &Impl,
532532
TypeLoc::withoutLoc(rawTy), enumDecl);
533533
getterDecl->setImplicit();
534534
getterDecl->setIsObjC(false);
535+
getterDecl->setIsDynamic(false);
535536

536537
auto type = ParameterList::getFullInterfaceType(rawTy, params, C);
537538

@@ -617,6 +618,7 @@ static AccessorDecl *makeStructRawValueGetter(
617618
TypeLoc::withoutLoc(computedType), structDecl);
618619
getterDecl->setImplicit();
619620
getterDecl->setIsObjC(false);
621+
getterDecl->setIsDynamic(false);
620622

621623
auto type = ParameterList::getFullInterfaceType(computedType, params, C);
622624

@@ -687,6 +689,7 @@ static AccessorDecl *makeFieldGetterDecl(ClangImporter::Implementation &Impl,
687689
TypeLoc::withoutLoc(getterType), importedDecl, clangNode);
688690
getterDecl->setAccess(AccessLevel::Public);
689691
getterDecl->setIsObjC(false);
692+
getterDecl->setIsDynamic(false);
690693

691694
auto type = ParameterList::getFullInterfaceType(getterType, params, C);
692695
getterDecl->setInterfaceType(type);
@@ -729,6 +732,7 @@ static AccessorDecl *makeFieldSetterDecl(ClangImporter::Implementation &Impl,
729732
/*GenericParams=*/nullptr, params,
730733
TypeLoc::withoutLoc(voidTy), importedDecl, clangNode);
731734
setterDecl->setIsObjC(false);
735+
setterDecl->setIsDynamic(false);
732736

733737
auto type = ParameterList::getFullInterfaceType(voidTy, params, C);
734738
setterDecl->setInterfaceType(type);
@@ -1650,6 +1654,7 @@ buildSubscriptGetterDecl(ClangImporter::Implementation &Impl,
16501654
if (auto objcAttr = getter->getAttrs().getAttribute<ObjCAttr>())
16511655
thunk->getAttrs().add(objcAttr->clone(C));
16521656
thunk->setIsObjC(getter->isObjC());
1657+
thunk->setIsDynamic(getter->isDynamic());
16531658
// FIXME: Should we record thunks?
16541659

16551660
return thunk;
@@ -1720,6 +1725,7 @@ buildSubscriptSetterDecl(ClangImporter::Implementation &Impl,
17201725
if (auto objcAttr = setter->getAttrs().getAttribute<ObjCAttr>())
17211726
thunk->getAttrs().add(objcAttr->clone(C));
17221727
thunk->setIsObjC(setter->isObjC());
1728+
thunk->setIsDynamic(setter->isDynamic());
17231729

17241730
return thunk;
17251731
}
@@ -1887,6 +1893,7 @@ static bool addErrorDomain(NominalTypeDecl *swiftDecl,
18871893
getterDecl->setInterfaceType(toStringTy);
18881894
getterDecl->setValidationToChecked();
18891895
getterDecl->setIsObjC(false);
1896+
getterDecl->setIsDynamic(false);
18901897

18911898
swiftDecl->addMember(errorDomainPropertyDecl);
18921899
swiftDecl->addMember(getterDecl);
@@ -3450,6 +3457,7 @@ namespace {
34503457
name, dc->mapTypeIntoContext(type), dc);
34513458
result->setInterfaceType(type);
34523459
result->setIsObjC(false);
3460+
result->setIsDynamic(false);
34533461
Impl.recordImplicitUnwrapForDecl(result,
34543462
importedType.isImplicitlyUnwrapped());
34553463

@@ -3585,6 +3593,7 @@ namespace {
35853593
result->setInterfaceType(type);
35863594
result->setValidationToChecked();
35873595
result->setIsObjC(false);
3596+
result->setIsDynamic(false);
35883597
Impl.recordImplicitUnwrapForDecl(result,
35893598
importedType.isImplicitlyUnwrapped());
35903599

@@ -3679,6 +3688,7 @@ namespace {
36793688
Impl.importSourceLoc(decl->getLocation()),
36803689
name, dc->mapTypeIntoContext(type), dc);
36813690
result->setIsObjC(false);
3691+
result->setIsDynamic(false);
36823692
result->setInterfaceType(type);
36833693
Impl.recordImplicitUnwrapForDecl(result,
36843694
importedType.isImplicitlyUnwrapped());
@@ -3765,6 +3775,7 @@ namespace {
37653775
Impl.importSourceLoc(decl->getLocation()),
37663776
name, dc->mapTypeIntoContext(type), dc);
37673777
result->setIsObjC(false);
3778+
result->setIsDynamic(false);
37683779
result->setInterfaceType(type);
37693780
Impl.recordImplicitUnwrapForDecl(result,
37703781
importedType.isImplicitlyUnwrapped());
@@ -3827,6 +3838,7 @@ namespace {
38273838
/*implicitName=*/true));
38283839
}
38293840
decl->setIsObjC(true);
3841+
decl->setIsDynamic(true);
38303842

38313843
// If the declaration we attached the 'objc' attribute to is within a
38323844
// class, record it in the class.
@@ -4831,52 +4843,54 @@ namespace {
48314843
if (shouldImportPropertyAsAccessors(decl))
48324844
return nullptr;
48334845

4834-
// Check whether there is a function with the same name as this
4835-
// property. If so, suppress the property; the user will have to use
4836-
// the methods directly, to avoid ambiguities.
4837-
Type containerTy = dc->getDeclaredInterfaceType();
4838-
Type lookupContextTy = containerTy;
4839-
if (auto *classDecl = dyn_cast<ClassDecl>(dc)) {
4840-
// If we're importing into the primary @interface for something, as
4841-
// opposed to an extension, make sure we don't try to load any
4842-
// categories...by just looking into the super type.
4843-
lookupContextTy = classDecl->getSuperclass();
4844-
}
4845-
48464846
VarDecl *overridden = nullptr;
4847-
if (lookupContextTy) {
4848-
SmallVector<ValueDecl *, 2> lookup;
4849-
dc->lookupQualified(lookupContextTy, name,
4850-
NL_QualifiedDefault | NL_KnownNoDependency,
4851-
Impl.getTypeResolver(), lookup);
4852-
for (auto result : lookup) {
4853-
if (isa<FuncDecl>(result) &&
4854-
result->isInstanceMember() == decl->isInstanceProperty() &&
4855-
result->getFullName().getArgumentNames().empty())
4856-
return nullptr;
4847+
if (dc->getAsClassOrClassExtensionContext()) {
4848+
// Check whether there is a function with the same name as this
4849+
// property. If so, suppress the property; the user will have to use
4850+
// the methods directly, to avoid ambiguities.
4851+
Type containerTy = dc->getDeclaredInterfaceType();
4852+
Type lookupContextTy = containerTy;
4853+
if (auto *classDecl = dyn_cast<ClassDecl>(dc)) {
4854+
// If we're importing into the primary @interface for something, as
4855+
// opposed to an extension, make sure we don't try to load any
4856+
// categories...by just looking into the super type.
4857+
lookupContextTy = classDecl->getSuperclass();
4858+
}
48574859

4858-
if (auto var = dyn_cast<VarDecl>(result)) {
4859-
// If the selectors of the getter match in Objective-C, we have an
4860-
// override.
4861-
if (var->isInstanceMember() == decl->isInstanceProperty() &&
4862-
var->getObjCGetterSelector() ==
4863-
Impl.importSelector(decl->getGetterName()))
4864-
overridden = var;
4860+
if (lookupContextTy) {
4861+
SmallVector<ValueDecl *, 2> lookup;
4862+
dc->lookupQualified(lookupContextTy, name,
4863+
NL_QualifiedDefault | NL_KnownNoDependency,
4864+
Impl.getTypeResolver(), lookup);
4865+
for (auto result : lookup) {
4866+
if (isa<FuncDecl>(result) &&
4867+
result->isInstanceMember() == decl->isInstanceProperty() &&
4868+
result->getFullName().getArgumentNames().empty())
4869+
return nullptr;
4870+
4871+
if (auto var = dyn_cast<VarDecl>(result)) {
4872+
// If the selectors of the getter match in Objective-C, we have an
4873+
// override.
4874+
if (var->isInstanceMember() == decl->isInstanceProperty() &&
4875+
var->getObjCGetterSelector() ==
4876+
Impl.importSelector(decl->getGetterName()))
4877+
overridden = var;
4878+
}
48654879
}
48664880
}
4867-
}
48684881

4869-
if (overridden) {
4870-
const DeclContext *overrideContext = overridden->getDeclContext();
4871-
// It's okay to compare interface types directly because Objective-C
4872-
// does not have constrained extensions.
4873-
if (overrideContext != dc && overridden->hasClangNode() &&
4874-
overrideContext->getDeclaredInterfaceType()->isEqual(containerTy)) {
4875-
// We've encountered a redeclaration of the property.
4876-
// HACK: Just update the original declaration instead of importing a
4877-
// second property.
4878-
handlePropertyRedeclaration(overridden, decl);
4879-
return nullptr;
4882+
if (overridden) {
4883+
const DeclContext *overrideContext = overridden->getDeclContext();
4884+
// It's okay to compare interface types directly because Objective-C
4885+
// does not have constrained extensions.
4886+
if (overrideContext != dc && overridden->hasClangNode() &&
4887+
overrideContext->getDeclaredInterfaceType()->isEqual(containerTy)) {
4888+
// We've encountered a redeclaration of the property.
4889+
// HACK: Just update the original declaration instead of importing a
4890+
// second property.
4891+
handlePropertyRedeclaration(overridden, decl);
4892+
return nullptr;
4893+
}
48804894
}
48814895
}
48824896

@@ -4941,6 +4955,7 @@ namespace {
49414955
OptionalAttr(/*implicit*/false));
49424956
// FIXME: Handle IBOutletCollection.
49434957

4958+
// Only record overrides of class members.
49444959
if (overridden) {
49454960
result->setOverriddenDecl(overridden);
49464961
getter->setOverriddenDecl(overridden->getGetter());
@@ -5685,6 +5700,7 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer(
56855700
importedType.isImplicitlyUnwrapped());
56865701
result->setOverriddenDecls({ });
56875702
result->setIsObjC(false);
5703+
result->setIsDynamic(false);
56885704

56895705
finishFuncDecl(decl, result);
56905706
if (correctSwiftName)
@@ -5932,6 +5948,8 @@ SwiftDeclConverter::getImplicitProperty(ImportedName importedName,
59325948
propertyName, dc->mapTypeIntoContext(swiftPropertyType), dc);
59335949
property->setInterfaceType(swiftPropertyType);
59345950
property->setIsObjC(false);
5951+
property->setIsDynamic(false);
5952+
59355953
Impl.recordImplicitUnwrapForDecl(property,
59365954
importedType.isImplicitlyUnwrapped());
59375955

@@ -8328,6 +8346,7 @@ ClangImporter::Implementation::createConstant(Identifier name, DeclContext *dc,
83288346

83298347
var->setInterfaceType(type);
83308348
var->setIsObjC(false);
8349+
var->setIsDynamic(false);
83318350

83328351
// Form the argument patterns.
83338352
SmallVector<ParameterList*, 3> getterArgs;
@@ -8363,6 +8382,7 @@ ClangImporter::Implementation::createConstant(Identifier name, DeclContext *dc,
83638382
func->setValidationToChecked();
83648383
func->setImplicit();
83658384
func->setIsObjC(false);
8385+
func->setIsDynamic(false);
83668386

83678387
// If we're not done type checking, build the getter body.
83688388
if (!hasFinishedTypeChecking()) {
@@ -8440,6 +8460,7 @@ createUnavailableDecl(Identifier name, DeclContext *dc, Type type,
84408460
/*IsCaptureList*/false,
84418461
SourceLoc(), name, type, dc);
84428462
var->setIsObjC(false);
8463+
var->setIsDynamic(false);
84438464
var->setInterfaceType(type);
84448465
markUnavailable(var, UnavailableMessage);
84458466

‎test/ClangImporter/Inputs/restated-protocol-requirement.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
@protocol P
2+
@property (nonatomic, copy, readonly, nullable) id property;
3+
- (void) method;
4+
- (id)objectForKeyedSubscript:(id)subscript;
5+
- (instancetype)initWithFoo:(id)foo;
6+
@end
7+
8+
@protocol Q <P>
9+
@property (nonatomic, copy, readonly, nullable) id property;
10+
- (void) method;
11+
- (id)objectForKeyedSubscript:(id)subscript;
12+
- (instancetype)initWithFoo:(id)foo;
13+
@end
14+

‎test/ClangImporter/restated-protocol-requirement.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// RUN: %target-swift-frontend -typecheck -import-objc-header %S/Inputs/restated-protocol-requirement.h %s
2+
3+
// REQUIRES: objc_interop
4+
5+
func giveMeAQ(_ q: Q) {
6+
_ = q.property
7+
_ = q.method()
8+
_ = q[q]
9+
_ = type(of: q).init(foo: q)
10+
}

0 commit comments

Comments
 (0)