From 915e215a3d50381b8571aa3e30b03e15551c6de3 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Fri, 5 Jun 2020 14:38:27 -0700 Subject: [PATCH] [ClangImporter] Make sure that inherited convenience constructors are included in members of `IterableDeclContext` Previously inherited constructors would be skipped from added in member list, depending on the order of request evaluator calls. This was a regression compared to swift 5.2 --- lib/ClangImporter/ImportDecl.cpp | 27 +- .../api-digester/Inputs/Foo-new-version/foo.h | 10 + test/api-digester/Inputs/Foo/foo.h | 10 + .../Outputs/clang-module-dump.txt | 334 ++++++++++++++++++ 4 files changed, 371 insertions(+), 10 deletions(-) diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 5f971d0872e63..a2726dd6c4c29 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -4313,12 +4313,12 @@ namespace { // "raw" name will be imported as unavailable with a more helpful and // specific message. ++NumFactoryMethodsAsInitializers; - bool redundant = false; + ConstructorDecl *existing = nullptr; auto result = importConstructor(decl, dc, false, importedName.getInitKind(), /*required=*/false, selector, importedName, {decl->param_begin(), decl->param_size()}, - decl->isVariadic(), redundant); + decl->isVariadic(), existing); if (!isActiveSwiftVersion() && result) markAsVariant(result, *correctSwiftName); @@ -4562,7 +4562,7 @@ namespace { ImportedName importedName, ArrayRef args, bool variadic, - bool &redundant); + ConstructorDecl *&existing); void recordObjCOverride(SubscriptDecl *subscript); @@ -6241,11 +6241,11 @@ ConstructorDecl *SwiftDeclConverter::importConstructor( variadic = false; } - bool redundant; + ConstructorDecl *existing; auto result = importConstructor(objcMethod, dc, implicit, kind.getValueOr(importedName.getInitKind()), required, selector, importedName, params, - variadic, redundant); + variadic, existing); // If this is a compatibility stub, mark it as such. if (result && correctSwiftName) @@ -6357,8 +6357,8 @@ ConstructorDecl *SwiftDeclConverter::importConstructor( const clang::ObjCMethodDecl *objcMethod, const DeclContext *dc, bool implicit, CtorInitializerKind kind, bool required, ObjCSelector selector, ImportedName importedName, ArrayRef args, - bool variadic, bool &redundant) { - redundant = false; + bool variadic, ConstructorDecl *&existing) { + existing = nullptr; // Figure out the type of the container. auto ownerNominal = dc->getSelfNominalTypeDecl(); @@ -6458,7 +6458,7 @@ ConstructorDecl *SwiftDeclConverter::importConstructor( // Otherwise, we shouldn't create a new constructor, because // it will be no better than the existing one. - redundant = true; + existing = ctor; return nullptr; } @@ -7395,19 +7395,26 @@ void SwiftDeclConverter::importInheritedConstructors( !correctSwiftName && "Import inherited initializers never references correctSwiftName"); importedName.setHasCustomName(); - bool redundant; + ConstructorDecl *existing; if (auto newCtor = importConstructor(objcMethod, classDecl, /*implicit=*/true, ctor->getInitKind(), /*required=*/false, ctor->getObjCSelector(), importedName, objcMethod->parameters(), - objcMethod->isVariadic(), redundant)) { + objcMethod->isVariadic(), existing)) { // If this is a compatibility stub, mark it as such. if (correctSwiftName) markAsVariant(newCtor, *correctSwiftName); Impl.importAttributes(objcMethod, newCtor, curObjCClass); newMembers.push_back(newCtor); + } else if (existing && existing->getClangDecl()) { + // Check that the existing constructor the prevented new creation is + // really an inherited factory initializer and not a class member. + auto existingMD = cast(existing->getClangDecl()); + if (existingMD->getClassInterface() != curObjCClass) { + newMembers.push_back(existing); + } } continue; } diff --git a/test/api-digester/Inputs/Foo-new-version/foo.h b/test/api-digester/Inputs/Foo-new-version/foo.h index b307b23510839..536a81f66ee9e 100644 --- a/test/api-digester/Inputs/Foo-new-version/foo.h +++ b/test/api-digester/Inputs/Foo-new-version/foo.h @@ -17,3 +17,13 @@ @interface ClangInterface: NSObject - (void)someFunction; @end + +@interface PhotoSettings: NSObject ++ (instancetype)photoSettingsWithFormat:(int)format; ++ (instancetype)photoSettingsWithNumber:(int)number; +@end + +@interface PhotoBracketSettings : PhotoSettings ++ (instancetype)photoBracketSettingsWithRawPixelFormatType:(int)rawPixelFormatType processedFormat:(int)processedFormat; ++ (instancetype)photoBracketSettingsWithNumber:(int)number; +@end diff --git a/test/api-digester/Inputs/Foo/foo.h b/test/api-digester/Inputs/Foo/foo.h index 93bcc41eb96be..54922ce83ac4e 100644 --- a/test/api-digester/Inputs/Foo/foo.h +++ b/test/api-digester/Inputs/Foo/foo.h @@ -11,3 +11,13 @@ @interface ClangInterface: NSObject - (void)someFunction; @end + +@interface PhotoSettings: NSObject ++ (instancetype)photoSettingsWithFormat:(int)format; ++ (instancetype)photoSettingsWithNumber:(int)number; +@end + +@interface PhotoBracketSettings : PhotoSettings ++ (instancetype)photoBracketSettingsWithRawPixelFormatType:(int)rawPixelFormatType processedFormat:(int)processedFormat; ++ (instancetype)photoBracketSettingsWithNumber:(int)number; +@end diff --git a/test/api-digester/Outputs/clang-module-dump.txt b/test/api-digester/Outputs/clang-module-dump.txt index d6d9a22418752..9fcffbebb038b 100644 --- a/test/api-digester/Outputs/clang-module-dump.txt +++ b/test/api-digester/Outputs/clang-module-dump.txt @@ -201,6 +201,340 @@ "ObjC", "Dynamic" ] + }, + { + "kind": "TypeDecl", + "name": "PhotoBracketSettings", + "printedName": "PhotoBracketSettings", + "children": [ + { + "kind": "Constructor", + "name": "init", + "printedName": "init(rawPixelFormatType:processedFormat:)", + "children": [ + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Foo.PhotoBracketSettings?", + "children": [ + { + "kind": "TypeNominal", + "name": "PhotoBracketSettings", + "printedName": "Foo.PhotoBracketSettings", + "usr": "c:objc(cs)PhotoBracketSettings" + } + ], + "usr": "s:Sq" + }, + { + "kind": "TypeNominal", + "name": "Int32", + "printedName": "Swift.Int32", + "usr": "s:s5Int32V" + }, + { + "kind": "TypeNominal", + "name": "Int32", + "printedName": "Swift.Int32", + "usr": "s:s5Int32V" + } + ], + "declKind": "Constructor", + "usr": "c:objc(cs)PhotoBracketSettings(cm)photoBracketSettingsWithRawPixelFormatType:processedFormat:", + "moduleName": "Foo", + "objc_name": "photoBracketSettingsWithRawPixelFormatType:processedFormat:", + "declAttributes": [ + "ObjC", + "Dynamic" + ], + "init_kind": "ConvenienceFactory" + }, + { + "kind": "Constructor", + "name": "init", + "printedName": "init(number:)", + "children": [ + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Foo.PhotoBracketSettings?", + "children": [ + { + "kind": "TypeNominal", + "name": "PhotoBracketSettings", + "printedName": "Foo.PhotoBracketSettings", + "usr": "c:objc(cs)PhotoBracketSettings" + } + ], + "usr": "s:Sq" + }, + { + "kind": "TypeNominal", + "name": "Int32", + "printedName": "Swift.Int32", + "usr": "s:s5Int32V" + } + ], + "declKind": "Constructor", + "usr": "c:objc(cs)PhotoBracketSettings(cm)photoBracketSettingsWithNumber:", + "moduleName": "Foo", + "objc_name": "photoBracketSettingsWithNumber:", + "declAttributes": [ + "ObjC", + "Dynamic" + ], + "init_kind": "ConvenienceFactory" + }, + { + "kind": "Constructor", + "name": "init", + "printedName": "init(format:)", + "children": [ + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Foo.PhotoBracketSettings?", + "children": [ + { + "kind": "TypeNominal", + "name": "PhotoBracketSettings", + "printedName": "Foo.PhotoBracketSettings", + "usr": "c:objc(cs)PhotoBracketSettings" + } + ], + "usr": "s:Sq" + }, + { + "kind": "TypeNominal", + "name": "Int32", + "printedName": "Swift.Int32", + "usr": "s:s5Int32V" + } + ], + "declKind": "Constructor", + "usr": "c:objc(cs)PhotoSettings(cm)photoSettingsWithFormat:", + "moduleName": "Foo", + "overriding": true, + "implicit": true, + "objc_name": "photoSettingsWithFormat:", + "declAttributes": [ + "Override", + "ObjC", + "Dynamic" + ], + "init_kind": "ConvenienceFactory" + }, + { + "kind": "Constructor", + "name": "init", + "printedName": "init()", + "children": [ + { + "kind": "TypeNominal", + "name": "PhotoBracketSettings", + "printedName": "Foo.PhotoBracketSettings", + "usr": "c:objc(cs)PhotoBracketSettings" + } + ], + "declKind": "Constructor", + "usr": "c:objc(cs)NSObject(im)init", + "moduleName": "Foo", + "overriding": true, + "implicit": true, + "objc_name": "init", + "declAttributes": [ + "Override", + "ObjC", + "Dynamic" + ], + "init_kind": "Designated" + } + ], + "declKind": "Class", + "usr": "c:objc(cs)PhotoBracketSettings", + "moduleName": "Foo", + "isOpen": true, + "objc_name": "PhotoBracketSettings", + "declAttributes": [ + "ObjC", + "Dynamic" + ], + "superclassUsr": "c:objc(cs)PhotoSettings", + "inheritsConvenienceInitializers": true, + "superclassNames": [ + "Foo.PhotoSettings", + "ObjectiveC.NSObject" + ], + "conformances": [ + { + "kind": "Conformance", + "name": "NSObjectProtocol", + "printedName": "NSObjectProtocol", + "usr": "c:objc(pl)NSObject" + }, + { + "kind": "Conformance", + "name": "Equatable", + "printedName": "Equatable", + "usr": "s:SQ" + }, + { + "kind": "Conformance", + "name": "Hashable", + "printedName": "Hashable", + "usr": "s:SH" + }, + { + "kind": "Conformance", + "name": "CVarArg", + "printedName": "CVarArg", + "usr": "s:s7CVarArgP" + } + ] + }, + { + "kind": "TypeDecl", + "name": "PhotoSettings", + "printedName": "PhotoSettings", + "children": [ + { + "kind": "Constructor", + "name": "init", + "printedName": "init(format:)", + "children": [ + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Foo.PhotoSettings?", + "children": [ + { + "kind": "TypeNominal", + "name": "PhotoSettings", + "printedName": "Foo.PhotoSettings", + "usr": "c:objc(cs)PhotoSettings" + } + ], + "usr": "s:Sq" + }, + { + "kind": "TypeNominal", + "name": "Int32", + "printedName": "Swift.Int32", + "usr": "s:s5Int32V" + } + ], + "declKind": "Constructor", + "usr": "c:objc(cs)PhotoSettings(cm)photoSettingsWithFormat:", + "moduleName": "Foo", + "objc_name": "photoSettingsWithFormat:", + "declAttributes": [ + "ObjC", + "Dynamic" + ], + "init_kind": "ConvenienceFactory" + }, + { + "kind": "Constructor", + "name": "init", + "printedName": "init(number:)", + "children": [ + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Foo.PhotoSettings?", + "children": [ + { + "kind": "TypeNominal", + "name": "PhotoSettings", + "printedName": "Foo.PhotoSettings", + "usr": "c:objc(cs)PhotoSettings" + } + ], + "usr": "s:Sq" + }, + { + "kind": "TypeNominal", + "name": "Int32", + "printedName": "Swift.Int32", + "usr": "s:s5Int32V" + } + ], + "declKind": "Constructor", + "usr": "c:objc(cs)PhotoSettings(cm)photoSettingsWithNumber:", + "moduleName": "Foo", + "objc_name": "photoSettingsWithNumber:", + "declAttributes": [ + "ObjC", + "Dynamic" + ], + "init_kind": "ConvenienceFactory" + }, + { + "kind": "Constructor", + "name": "init", + "printedName": "init()", + "children": [ + { + "kind": "TypeNominal", + "name": "PhotoSettings", + "printedName": "Foo.PhotoSettings", + "usr": "c:objc(cs)PhotoSettings" + } + ], + "declKind": "Constructor", + "usr": "c:objc(cs)NSObject(im)init", + "moduleName": "Foo", + "overriding": true, + "implicit": true, + "objc_name": "init", + "declAttributes": [ + "Override", + "ObjC", + "Dynamic" + ], + "init_kind": "Designated" + } + ], + "declKind": "Class", + "usr": "c:objc(cs)PhotoSettings", + "moduleName": "Foo", + "isOpen": true, + "objc_name": "PhotoSettings", + "declAttributes": [ + "ObjC", + "Dynamic" + ], + "superclassUsr": "c:objc(cs)NSObject", + "inheritsConvenienceInitializers": true, + "superclassNames": [ + "ObjectiveC.NSObject" + ], + "conformances": [ + { + "kind": "Conformance", + "name": "NSObjectProtocol", + "printedName": "NSObjectProtocol", + "usr": "c:objc(pl)NSObject" + }, + { + "kind": "Conformance", + "name": "Equatable", + "printedName": "Equatable", + "usr": "s:SQ" + }, + { + "kind": "Conformance", + "name": "Hashable", + "printedName": "Hashable", + "usr": "s:SH" + }, + { + "kind": "Conformance", + "name": "CVarArg", + "printedName": "CVarArg", + "usr": "s:s7CVarArgP" + } + ] } ], "json_format_version": 6