Skip to content

Don't set overrides on imported protocol requirements [5.0] #18376

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 2 commits into from
Aug 1, 2018
Merged

Don't set overrides on imported protocol requirements [5.0] #18376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
faf31d8
ClangImporter: Set the 'is dynamic' bit explicitly on synthesized dec…
slavapestov Jul 30, 2018
e440e1d
ClangImporter: Only set overrides on class members
slavapestov Jul 30, 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
103 changes: 62 additions & 41 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ static AccessorDecl *makeEnumRawValueGetter(ClangImporter::Implementation &Impl,
TypeLoc::withoutLoc(rawTy), enumDecl);
getterDecl->setImplicit();
getterDecl->setIsObjC(false);
getterDecl->setIsDynamic(false);

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

Expand Down Expand Up @@ -617,6 +618,7 @@ static AccessorDecl *makeStructRawValueGetter(
TypeLoc::withoutLoc(computedType), structDecl);
getterDecl->setImplicit();
getterDecl->setIsObjC(false);
getterDecl->setIsDynamic(false);

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

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

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

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

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

return thunk;
}
Expand Down Expand Up @@ -1887,6 +1893,7 @@ static bool addErrorDomain(NominalTypeDecl *swiftDecl,
getterDecl->setInterfaceType(toStringTy);
getterDecl->setValidationToChecked();
getterDecl->setIsObjC(false);
getterDecl->setIsDynamic(false);

swiftDecl->addMember(errorDomainPropertyDecl);
swiftDecl->addMember(getterDecl);
Expand Down Expand Up @@ -3450,6 +3457,7 @@ namespace {
name, dc->mapTypeIntoContext(type), dc);
result->setInterfaceType(type);
result->setIsObjC(false);
result->setIsDynamic(false);
Impl.recordImplicitUnwrapForDecl(result,
importedType.isImplicitlyUnwrapped());

Expand Down Expand Up @@ -3585,6 +3593,7 @@ namespace {
result->setInterfaceType(type);
result->setValidationToChecked();
result->setIsObjC(false);
result->setIsDynamic(false);
Impl.recordImplicitUnwrapForDecl(result,
importedType.isImplicitlyUnwrapped());

Expand Down Expand Up @@ -3679,6 +3688,7 @@ namespace {
Impl.importSourceLoc(decl->getLocation()),
name, dc->mapTypeIntoContext(type), dc);
result->setIsObjC(false);
result->setIsDynamic(false);
result->setInterfaceType(type);
Impl.recordImplicitUnwrapForDecl(result,
importedType.isImplicitlyUnwrapped());
Expand Down Expand Up @@ -3765,6 +3775,7 @@ namespace {
Impl.importSourceLoc(decl->getLocation()),
name, dc->mapTypeIntoContext(type), dc);
result->setIsObjC(false);
result->setIsDynamic(false);
result->setInterfaceType(type);
Impl.recordImplicitUnwrapForDecl(result,
importedType.isImplicitlyUnwrapped());
Expand Down Expand Up @@ -3827,6 +3838,7 @@ namespace {
/*implicitName=*/true));
}
decl->setIsObjC(true);
decl->setIsDynamic(true);

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

// Check whether there is a function with the same name as this
// property. If so, suppress the property; the user will have to use
// the methods directly, to avoid ambiguities.
Type containerTy = dc->getDeclaredInterfaceType();
Type lookupContextTy = containerTy;
if (auto *classDecl = dyn_cast<ClassDecl>(dc)) {
// If we're importing into the primary @interface for something, as
// opposed to an extension, make sure we don't try to load any
// categories...by just looking into the super type.
lookupContextTy = classDecl->getSuperclass();
}

VarDecl *overridden = nullptr;
if (lookupContextTy) {
SmallVector<ValueDecl *, 2> lookup;
dc->lookupQualified(lookupContextTy, name,
NL_QualifiedDefault | NL_KnownNoDependency,
Impl.getTypeResolver(), lookup);
for (auto result : lookup) {
if (isa<FuncDecl>(result) &&
result->isInstanceMember() == decl->isInstanceProperty() &&
result->getFullName().getArgumentNames().empty())
return nullptr;
if (dc->getAsClassOrClassExtensionContext()) {
// Check whether there is a function with the same name as this
// property. If so, suppress the property; the user will have to use
// the methods directly, to avoid ambiguities.
Type containerTy = dc->getDeclaredInterfaceType();
Type lookupContextTy = containerTy;
if (auto *classDecl = dyn_cast<ClassDecl>(dc)) {
// If we're importing into the primary @interface for something, as
// opposed to an extension, make sure we don't try to load any
// categories...by just looking into the super type.
lookupContextTy = classDecl->getSuperclass();
}

if (auto var = dyn_cast<VarDecl>(result)) {
// If the selectors of the getter match in Objective-C, we have an
// override.
if (var->isInstanceMember() == decl->isInstanceProperty() &&
var->getObjCGetterSelector() ==
Impl.importSelector(decl->getGetterName()))
overridden = var;
if (lookupContextTy) {
SmallVector<ValueDecl *, 2> lookup;
dc->lookupQualified(lookupContextTy, name,
NL_QualifiedDefault | NL_KnownNoDependency,
Impl.getTypeResolver(), lookup);
for (auto result : lookup) {
if (isa<FuncDecl>(result) &&
result->isInstanceMember() == decl->isInstanceProperty() &&
result->getFullName().getArgumentNames().empty())
return nullptr;

if (auto var = dyn_cast<VarDecl>(result)) {
// If the selectors of the getter match in Objective-C, we have an
// override.
if (var->isInstanceMember() == decl->isInstanceProperty() &&
var->getObjCGetterSelector() ==
Impl.importSelector(decl->getGetterName()))
overridden = var;
}
}
}
}

if (overridden) {
const DeclContext *overrideContext = overridden->getDeclContext();
// It's okay to compare interface types directly because Objective-C
// does not have constrained extensions.
if (overrideContext != dc && overridden->hasClangNode() &&
overrideContext->getDeclaredInterfaceType()->isEqual(containerTy)) {
// We've encountered a redeclaration of the property.
// HACK: Just update the original declaration instead of importing a
// second property.
handlePropertyRedeclaration(overridden, decl);
return nullptr;
if (overridden) {
const DeclContext *overrideContext = overridden->getDeclContext();
// It's okay to compare interface types directly because Objective-C
// does not have constrained extensions.
if (overrideContext != dc && overridden->hasClangNode() &&
overrideContext->getDeclaredInterfaceType()->isEqual(containerTy)) {
// We've encountered a redeclaration of the property.
// HACK: Just update the original declaration instead of importing a
// second property.
handlePropertyRedeclaration(overridden, decl);
return nullptr;
}
}
}

Expand Down Expand Up @@ -4941,6 +4955,7 @@ namespace {
OptionalAttr(/*implicit*/false));
// FIXME: Handle IBOutletCollection.

// Only record overrides of class members.
if (overridden) {
result->setOverriddenDecl(overridden);
getter->setOverriddenDecl(overridden->getGetter());
Expand Down Expand Up @@ -5685,6 +5700,7 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer(
importedType.isImplicitlyUnwrapped());
result->setOverriddenDecls({ });
result->setIsObjC(false);
result->setIsDynamic(false);

finishFuncDecl(decl, result);
if (correctSwiftName)
Expand Down Expand Up @@ -5932,6 +5948,8 @@ SwiftDeclConverter::getImplicitProperty(ImportedName importedName,
propertyName, dc->mapTypeIntoContext(swiftPropertyType), dc);
property->setInterfaceType(swiftPropertyType);
property->setIsObjC(false);
property->setIsDynamic(false);

Impl.recordImplicitUnwrapForDecl(property,
importedType.isImplicitlyUnwrapped());

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

var->setInterfaceType(type);
var->setIsObjC(false);
var->setIsDynamic(false);

// Form the argument patterns.
SmallVector<ParameterList*, 3> getterArgs;
Expand Down Expand Up @@ -8363,6 +8382,7 @@ ClangImporter::Implementation::createConstant(Identifier name, DeclContext *dc,
func->setValidationToChecked();
func->setImplicit();
func->setIsObjC(false);
func->setIsDynamic(false);

// If we're not done type checking, build the getter body.
if (!hasFinishedTypeChecking()) {
Expand Down Expand Up @@ -8440,6 +8460,7 @@ createUnavailableDecl(Identifier name, DeclContext *dc, Type type,
/*IsCaptureList*/false,
SourceLoc(), name, type, dc);
var->setIsObjC(false);
var->setIsDynamic(false);
var->setInterfaceType(type);
markUnavailable(var, UnavailableMessage);

Expand Down
14 changes: 14 additions & 0 deletions test/ClangImporter/Inputs/restated-protocol-requirement.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
@protocol P
@property (nonatomic, copy, readonly, nullable) id property;
- (void) method;
- (id)objectForKeyedSubscript:(id)subscript;
- (instancetype)initWithFoo:(id)foo;
@end

@protocol Q <P>
@property (nonatomic, copy, readonly, nullable) id property;
- (void) method;
- (id)objectForKeyedSubscript:(id)subscript;
- (instancetype)initWithFoo:(id)foo;
@end

10 changes: 10 additions & 0 deletions test/ClangImporter/restated-protocol-requirement.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// RUN: %target-swift-frontend -typecheck -import-objc-header %S/Inputs/restated-protocol-requirement.h %s

// REQUIRES: objc_interop

func giveMeAQ(_ q: Q) {
_ = q.property
_ = q.method()
_ = q[q]
_ = type(of: q).init(foo: q)
}