Use the custom @objc name in the available attribute when generates ObjC headers #18081

pitiphong-p · 2018-07-19T18:34:12Z

Printing the custom @objc name in availability attributes in the generated Objective-C header files instead of printing the Swift name. This should help the compiler/fix-it to change the deprecated/unavailable API usage with the appropriate name

Resolves SR-8231.

pitiphong-p · 2018-07-19T18:42:06Z

I have a few questions for this PR before proceeding

Should I open this PR to the swift-4.2 branch instead?
I still don't know how to get a Decl for a class given by a name so I left the work for the case of a class and will work on it as soon as I know a way (https://github.com/apple/swift/pull/18081/files#diff-27516adf920dd85ca69f0e537e6913e6R1910)
I don't sure how we should handle a case where the developer has a whole class to replace another class and they mark the old class's members with a rename availability attribute (You can see an example here https://github.com/apple/swift/pull/18081/files#diff-d7d8a9d1d74c516efb2594723443b62dR294)

jrose-apple · 2018-07-19T18:52:34Z

Thanks, I'll review this soon. Answers to your questions:

PRs should always go to the master branch first. We're getting to the point where we need to not be taking arbitrary changes on 4.2, so this might not make it there.
Probably in this case you'd want to use UnqualifiedLookup. But that can totally be a follow-up patch.
I think it's fine not to worry about that for now and just fall back to the Swift name. That can be more follow-up work if you want to try later. (I think the general strategy there would be to do a lookup for the class first, followed by a lookup for the other method. But you'd probably only want to print that in a message, not the "renamed" attribute.)

varokas · 2018-07-19T19:02:15Z

lib/PrintAsObjC/PrintAsObjC.cpp

@@ -771,18 +771,18 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
               !FD->getAttrs().hasAttribute<DiscardableResultAttr>()) {
      os << " SWIFT_WARN_UNUSED_RESULT";
    }
-
+    


nit: preserve whitespaces? applies to line 779, 784, 785

varokas · 2018-07-19T19:04:05Z

test/PrintAsObjC/availability.swift

@@ -82,7 +106,7 @@

 @objc class Availability {
    @objc func alwaysAvailable() {}
-
+    


nit: perverse whitespaces?

varokas · 2018-07-19T19:15:09Z

lib/PrintAsObjC/PrintAsObjC.cpp

+              printEncodedString(renamedObjCRuntimeName, false);
+            } else {
+              printEncodedString(AvAttr->Rename, false);
+            }


nit: This is identical to line 923-931, and almost the same as 839-847. consider refactor this into a function?

I'm with varokas on this. I think you could push this all into findRenamedDecl and rename it to something like printRenameForDecl.

pitiphong-p · 2018-07-19T19:24:34Z

@jrose-apple thank you. I’ll work on the number 2 on tomorrow and May try the number 3 too

jrose-apple

This is looking great! I have a lot of small comments, but the general approach is good.

I'd also like to see a lot more tests:

Renames where there are multiple methods with the same full name but different parameter types.
Renames where the renamed-to method comes from Objective-C.
Renames where the renamed-to method is a class method instead of an instance method or vice versa.
Renames of initializers.
Renames of methods to properties or properties to methods.
Renames to things that just don't exist.

…and that's all I can think of for now. But worth testing, yeah? :-)

Thanks again for picking this up!

jrose-apple · 2018-07-21T02:22:45Z

lib/PrintAsObjC/PrintAsObjC.cpp

+              printEncodedString(renamedObjCRuntimeName, false);
+            } else {
+              printEncodedString(AvAttr->Rename, false);
+            }


I'm with varokas on this. I think you could push this all into findRenamedDecl and rename it to something like printRenameForDecl.

jrose-apple · 2018-07-21T02:23:26Z

lib/PrintAsObjC/PrintAsObjC.cpp

@@ -1884,6 +1906,46 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
    visitPart(RST->getReferentType(), optionalKind);
  }

+  Optional<ValueDecl *> findRenamedDecl(const swift::AvailableAttr *AvAttr, const swift::Decl *D) {


Nitpick: We have an 80-character column limit, but you can also drop the swift:: namespace whenever you're in a Swift source file.

jrose-apple · 2018-07-21T02:25:19Z

lib/PrintAsObjC/PrintAsObjC.cpp

+    Optional<ValueDecl *>renamedFuncDecl = None;
+
+    if (isa<ClassDecl>(D) || isa<ProtocolDecl>(D)) {
+      UnqualifiedLookup lookup(renamedDeclName.getBaseIdentifier(), declContext->getModuleScopeContext(), nullptr);


Nitpick: You can pass UnqualifiedLookup::Flags::TypeLookup to the "options" parameter here to make the lookup slightly more efficient.

jrose-apple · 2018-07-21T02:25:59Z

lib/PrintAsObjC/PrintAsObjC.cpp

+    auto renamedDeclName = renamedParsedDeclName.formDeclName(D->getASTContext());
+
+    auto declContext = D->getDeclContext();
+    Optional<ValueDecl *>renamedFuncDecl = None;


Nitpick: This isn't always a function now. :-) You can also probably use nullptr as your "not found" value and drop the Optional. It's a little more subtle, but a pretty common idiom in our C++ code.

jrose-apple · 2018-07-21T02:26:34Z

lib/PrintAsObjC/PrintAsObjC.cpp

+    if (!isa<ValueDecl>(D))
+      return None;
+
+    SmallVector<ValueDecl *, 4> lookupResults;


Nitpick: Might as well move this into the qualified lookup branch now!

jrose-apple · 2018-07-21T02:30:48Z

lib/PrintAsObjC/PrintAsObjC.cpp

+        if (candidate->getKind() == DeclKind::Func && isa<FuncDecl>(candidate)
+            && cast<FuncDecl>(candidate)->getParameterLists().size() !=
+            cast<FuncDecl>(D)->getParameterLists().size())
+          continue;


I think you're trying to check that the number of parameters is the same here? That's not a bad idea, but it's not getParameterLists() that you want. Fortunately Slava P just added a useful API for what you do want: getParameters().

Another good thing to check is whether they're both instance members.

THAT IS GREAT!!! Thank you Slava P :D

jrose-apple · 2018-07-21T02:32:25Z

lib/PrintAsObjC/PrintAsObjC.cpp

+        if (candidate->getKind() != D->getKind())
+          continue;
+
+        if (candidate->getKind() == DeclKind::Func && isa<FuncDecl>(candidate)


It seems like you can use isa for candidate too, no?

jrose-apple · 2018-07-21T02:34:38Z

lib/PrintAsObjC/PrintAsObjC.cpp

+                                   renamedDeclName, NL_QualifiedDefault, NULL,
+                                   lookupResults);
+      for (auto candidate : lookupResults) {
+        if (!shouldInclude(candidate))


Good idea here—easy way to filter for ObjC-compatible things only!

jrose-apple · 2018-07-21T02:34:58Z

lib/PrintAsObjC/PrintAsObjC.cpp

+          continue;
+
+        if (renamedFuncDecl)
+        /* TODO: Diagnose to compiler to tell the user that we found multiple candidates */;


I think it's okay to just fail in this case, at least for now. Incremental improvements are good!

How should I make it fail here? Just fallback to the name in the rename attribute?

Yeah, that's what it's doing now. We could also consider not putting anything in the "rename" attribute when we can't get an ObjC name, and rely on the message. But for now, let's keep the existing behavior as our fallback.

…s the ObjC header

…d protocols when generates the ObjC header

…lability

… Type lookup

…d not instance method

…d not instance method and vice versa

…d be instance members or otherwise

pitiphong-p · 2018-07-21T17:14:15Z

@jrose-apple I fixed and updated the code as you suggested. Also I added some test cases too

Renames where there are multiple methods with the same full name but different parameter types.
Renames where the renamed-to method comes from Objective-C.
Renames where the renamed-to method is a class method instead of an instance method or vice versa.
Renames of initializers.
Renames of methods to properties or properties to methods.
Renames to things that just don't exist.

However I do have a few questions.

Renames where the renamed-to method comes from Objective-C. So I need to have another .h file to test this, right?
Renames of methods to properties or properties to methods I'm not sure what's the expected behavior here. Should the finding fail and fall back to the old behavior?

Please have a look again :)

jrose-apple · 2018-07-24T00:23:55Z

Renames where the renamed-to method comes from Objective-C

Well, you could have a test that imports the real Foundation and subclasses or extends something there. (Ooh, that's another test case: "renames where the renamed-to method comes from a superclass".)

Renames of methods to properties or properties to methods

Yeah, I'm happy to call this a fallback case. We could arguably handle the method-to-property case by using the name of the getter, and the property-to-method case by finding a no-argument method, but again, those changes can come later.

jrose-apple · 2018-07-24T00:24:58Z

lib/PrintAsObjC/PrintAsObjC.cpp

+  void printRenameForDecl(const AvailableAttr *AvAttr, const Decl *D,
+                          bool includeQuotes) {
+    if (AvAttr->Rename.empty() && isa<ValueDecl>(D))
+      return ;


We've already checked that this is a ValueDecl at all call sites, so you can probably just drop that. (Also, stray space before the semicolon.)

jrose-apple · 2018-07-24T00:26:16Z

lib/PrintAsObjC/PrintAsObjC.cpp

+          continue;
+
+        if (renamedFuncDecl)
+        /* TODO: Diagnose to compiler to tell the user that we found multiple candidates */;


Still gotta do something here before we check this in. Just keeping track of a second boolean flag is probably fine.

So we should just fail here and fallback to the AvAttr->Rename here?

Yeah, the same thing we'd do for a "we found nothing that matches" case.

jrose-apple · 2018-07-24T00:26:44Z

lib/PrintAsObjC/PrintAsObjC.cpp

+    auto renamedDeclName = renamedParsedDeclName.formDeclName(D->getASTContext());
+
+    auto declContext = D->getDeclContext();
+    ValueDecl *renamedFuncDecl = nullptr;


Still not necessarily a function anymore. :-) Also, may as well mark this const.

Still not necessarily a function anymore I'm not sure what you mean here

jrose-apple · 2018-07-24T00:27:53Z

test/PrintAsObjC/availability.swift

@@ -45,11 +45,29 @@
 // CHECK-DAG: SWIFT_AVAILABILITY(ios_app_extension,unavailable)
 // CHECK-DAG: SWIFT_AVAILABILITY(tvos_app_extension,unavailable)
 // CHECK-DAG: SWIFT_AVAILABILITY(watchos_app_extension,unavailable)
+// CHECK-DAG: ;


I think it's okay to leave this out, but if you want to have it you should use CHECK-SAME rather than CHECK-DAG. The "DAG" suffix means it's reorderable with other CHECK-DAG lines immediately before and after this one.

Oh I just learn about CHECK-SAME, thanks

jrose-apple · 2018-07-24T00:28:42Z

test/PrintAsObjC/availability.swift

+// CHECK-NEXT: + (void)makeDeprecatedAvailabilityWithValue:(NSInteger)value SWIFT_DEPRECATED_MSG("use something else", "deprecatedAvailabilityWithValue:");
+// CHECK-NEXT: + (void)unavailableAvailabilityWithValue:(NSInteger)value;
+// CHECK-NEXT: + (void)makeUnavailableAvailabilityWithValue:(NSInteger)value
+// CHECK-DAG: SWIFT_UNAVAILABLE_MSG("'__makeUnavailableAvailability' has been renamed to 'unavailableAvailabilityWithValue:': use something else");


CHECK-SAME would be a better choice here too (since in this case there's only one thing to match).

jrose-apple · 2018-07-24T00:31:09Z

test/PrintAsObjC/availability.swift

+}
+
+@objc(SWTReplacementAvailableProtocol) protocol ReplacementAvailableProtocol {
+  @objc(methodReplacingInDeprecatedClassWithFirst:second:) func methodReplacingInDeprecatedClass(first: Int, second: Int) -> Void


I'll note that this is the same as the default name; you may want to pick something else.

jrose-apple · 2018-07-24T00:31:38Z

test/PrintAsObjC/availability.swift

+    @objc func deprecatedMethodRenamedToOverloadMethod1(first: Int, second: Int) {}
+
+    @objc(overloadMethodWithFirst:second:) func overloadMethod1(first: Int, second: Int) {}
+    @objc func overloadMethod2(first: Double, second: Double) {}


Wait, these aren't overloads if you give them different base names.

OH what a mistake!!!

jrose-apple · 2018-07-24T00:33:36Z

Oh, more possible test cases:

renaming to a method that does not have a custom Objective-C name (but is still exposed to Objective-C)
renaming to a method that is not exposed to Objective-C (maybe with an explicit @nonobjc)

…ame test case

pitiphong-p · 2018-07-24T07:57:37Z

Renames where there are multiple methods with the same full name but different parameter types.
Renames where the renamed-to method is a class method instead of an instance method or vice versa.
Renames of initializers.
Renames of methods to properties or properties to methods.
Renames to things that just don't exist.
Renaming to a method that is not exposed to Objective-C (maybe with an explicit @nonobjc)
Renaming to a method that does not have a custom Objective-C name (but is still exposed to Objective-C)
Renames where the renamed-to method comes from Objective-C.

@jrose-apple I updated the code and added more test cases as you suggested. Could you review it again?

However there is an error while I was trying to add a test case for Renames where the renamed-to method comes from Objective-C case. Here's a standard error output

Exit Code: 134

Command Output (stderr):

0 swiftc 0x000000010e28f968 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1 swiftc 0x000000010e28eba7 llvm::sys::RunSignalHandlers() + 39
2 swiftc 0x000000010e28ffe2 SignalHandler(int) + 258
3 libsystem_platform.dylib 0x00007fff5c583f5a _sigtramp + 26
4 libsystem_platform.dylib 0x0000000120dae314 _sigtramp + 3296895956
5 libsystem_c.dylib 0x00007fff5c3211ae abort + 127
6 swiftc 0x000000010b9d61df swift::ModuleFile::readConformance(llvm::BitstreamCursor&, swift::GenericEnvironment*) + 2207
7 swiftc 0x000000010b9d5e6e swift::ModuleFile::readConformance(llvm::BitstreamCursor&, swift::GenericEnvironment*) + 1326
8 swiftc 0x000000010b9ecc9d swift::ModuleFile::loadAllConformances(swift::Decl const*, unsigned long long, llvm::SmallVectorImplswift::ProtocolConformance*&) + 237
9 swiftc 0x000000010bc4be11 swift::ConformanceLookupTable::updateLookupTable(swift::NominalTypeDecl*, swift::ConformanceLookupTable::ConformanceStage, swift::LazyResolver*) + 193
10 swiftc 0x000000010bc4c651 swift::ConformanceLookupTable::updateLookupTable(swift::NominalTypeDecl*, swift::ConformanceLookupTable::ConformanceStage, swift::LazyResolver*) + 2305
11 swiftc 0x000000010bc4c1a5 swift::ConformanceLookupTable::updateLookupTable(swift::NominalTypeDecl*, swift::ConformanceLookupTable::ConformanceStage, swift::LazyResolver*) + 1109
12 swiftc 0x000000010bc4fa37 swift::ConformanceLookupTable::lookupConformances(swift::NominalTypeDecl*, swift::DeclContext*, swift::LazyResolver*, swift::ConformanceLookupKind, llvm::SmallVectorImplswift::ProtocolDecl*, llvm::SmallVectorImplswift::ProtocolConformance*, llvm::SmallVectorImplswift::ConformanceDiagnostic) + 55
13 swiftc 0x000000010bd0f323 swift::DeclContext::getLocalProtocols(swift::ConformanceLookupKind, llvm::SmallVectorImplswift::ConformanceDiagnostic, bool) const + 131
14 swiftc 0x000000010b5d21dd swift::printAsObjC(llvm::raw_ostream&, swift::ModuleDecl*, llvm::StringRef, swift::AccessLevel) + 3965
15 swiftc 0x000000010aa64fe2 printAsObjCIfNeeded(llvm::StringRef, swift::ModuleDecl*, llvm::StringRef, bool) + 450
16 swiftc 0x000000010aa60d0a performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) + 7146
17 swiftc 0x000000010aa5e0af swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 2943
18 swiftc 0x000000010aa18048 main + 1128
19 libdyld.dylib 0x00007fff5c275015 start + 1
20 libdyld.dylib 0x0000000000000017 start + 2748887043
Stack dump:
0. Program arguments: /Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/bin/swiftc -frontend -target x86_64-apple-macosx10.9 -module-cache-path /Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache -sdk /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -swift-version 4 -enable-source-import -sdk /Swift-Repo/swift/test/Inputs/clang-importer-sdk -I /Swift-Repo/swift/test/Inputs/clang-importer-sdk/swift-modules -parse-as-library /Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/test-macosx-x86_64/PrintAsObjC/Output/availability.swift.tmp/availability.swiftmodule -typecheck -emit-objc-header-path /Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/test-macosx-x86_64/PrintAsObjC/Output/availability.swift.tmp/availability.h -import-objc-header /Swift-Repo/swift/test/PrintAsObjC/../Inputs/empty.h -disable-objc-attr-requires-foundation-module

While generating Objective-C header

While loading conformances for 'SubClassOfBaseClass' in module 'availability'

While reading inherited conformance for type 'SubClassOfBaseClass'

While cross-referencing conformance for 'NSObject' in module 'ObjectiveC'

While ... to 'Equatable' in module 'Swift'

If you're seeing a crash here, check that your SDK and dependencies are at least as new as the versions used to build 'availability'
/Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/test-macosx-x86_64/PrintAsObjC/Output/availability.swift.script: line 5: 51065 Abort trap: 6 /Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/bin/swiftc -frontend -target x86_64-apple-macosx10.9 -module-cache-path '/Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache' -sdk '/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk' -swift-version 4 -enable-source-import -sdk '/Swift-Repo/swift/test/Inputs/clang-importer-sdk' -I '/Swift-Repo/swift/test/Inputs/clang-importer-sdk/swift-modules' -parse-as-library /Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/test-macosx-x86_64/PrintAsObjC/Output/availability.swift.tmp/availability.swiftmodule -typecheck -emit-objc-header-path /Swift-Repo/build/Ninja-RelWithDebInfoAssert/swift-macosx-x86_64/test-macosx-x86_64/PrintAsObjC/Output/availability.swift.tmp/availability.h -import-objc-header /Swift-Repo/swift/test/PrintAsObjC/../Inputs/empty.h -disable-objc-attr-requires-foundation-module

What I did was adding import Foundation and a new subclass of Coat class in the availability.swift

How could I fix this?

pitiphong-p · 2018-07-24T08:51:02Z

@jrose-apple I also implement the type following feature (e.g. renamed: "AnotherClass.thisIsAMethod(_:)") feature. You can have a look here pitiphong-p/swift@objc-rename-attribute...pitiphong-p:objc-rename-attribute-type-following

Should I include this change with this PR?

jrose-apple · 2018-07-25T02:57:03Z

What I did was adding import Foundation and a new subclass of Coat class in the availability.swift

How could I fix this?

Most of the PrintAsObjC tests, including availability.swift, use the "mock SDK" in test/Inputs/clang-importer-sdk/ in order to build faster, as well as decreasing the likelihood that a test will need to be updated for a new Xcode release. The catch, though, is that when you use the mock SDK, you have to set up appropriate equivalents for the overlays as well. You can see this in test/PrintAsObjC/classes.swift as "-enable-source-import hackaround".

There are a few options here: you can copy that workaround over to availability.swift, or you could make a new test file that uses the real SDK instead of the mock SDK (which would mostly just mean removing the "(mock-sdk: %clang-importer-sdk)" and "-disable-objc-attr-requires-foundation-module" from the RUN lines in availability.swift). Either one's fine with me.

jrose-apple

The code logic looks good, just a few tiny more changes I suggest. I'll look over all the tests I asked you to add tomorrow (Pacific time).

jrose-apple · 2018-07-25T02:57:45Z

lib/PrintAsObjC/PrintAsObjC.cpp

+    auto renamedDeclName = renamedParsedDeclName.formDeclName(D->getASTContext());
+
+    auto declContext = D->getDeclContext();
+    const ValueDecl *renamedFuncDecl = nullptr;


This still shouldn't be called "renamedFuncDecl" when it's very often not a function. :-)

Oh right, sorry that I missed that

jrose-apple · 2018-07-25T02:59:04Z

lib/PrintAsObjC/PrintAsObjC.cpp

+  void printRenameForDecl(const AvailableAttr *AvAttr, const ValueDecl *D,
+                          bool includeQuotes) {
+    if (AvAttr->Rename.empty())
+      return;


I think you can assert this rather than early-exiting. Any caller would already have to deal with the no-rename case differently.

(Sorry I didn't make this comment last time. I guess I was distracted by the formatting.)

OK, I'll change this to assert instead

jrose-apple · 2018-07-25T03:01:34Z

@jrose-apple I also implement the type following feature (e.g. renamed: "AnotherClass.thisIsAMethod(_:)") feature. You can have a look here pitiphong-p/swift@objc-rename-attribute...pitiphong-p:objc-rename-attribute-type-following

Should I include this change with this PR?

At this point I feel bad for dragging this one out, so let's not include that. That way we can merge this PR sooner.

pitiphong-p · 2018-07-25T09:47:51Z

At this point I feel bad for dragging this one out, so let's not include that. That way we can merge this PR sooner.

OK I'll open it as a new PR since I also have some ideas on improving this further :)

…cases

pitiphong-p · 2018-07-25T16:16:43Z

@jrose-apple I fixed the code and added test cases you asked for yesterday already. Please have a look again.
However I only added the deprecation test cases but will added the unavailable test cases after you finish checking the deprecated test cases.

P.S. I also added a new availability-real-sdk.swift for the renamed-to ObjC name test cases. I chose the removing the "(mock-sdk: %clang-importer-sdk)" and "-disable-objc-attr-requires-foundation-module" approach since another approach caused a build failure while running the test

jrose-apple

Tests all look good! Thanks for ticking all the boxes I asked for.

jrose-apple · 2018-07-25T16:38:01Z

test/PrintAsObjC/availability.swift

+// CHECK-NEXT: + (void)makeDeprecatedAvailabilityWithValue:(NSInteger)value SWIFT_DEPRECATED_MSG("use something else", "deprecatedAvailabilityWithValue:");
+// CHECK-NEXT: + (void)unavailableAvailabilityWithValue:(NSInteger)value;
+// CHECK-NEXT: + (void)makeUnavailableAvailabilityWithValue:(NSInteger)value
+// CHECK-SAME: SWIFT_UNAVAILABLE_MSG("'__makeUnavailableAvailability' has been renamed to 'unavailableAvailabilityWithValue:': use something else");


Hm, yeah, we should indeed fix the unavailable macro.

Fix the unavailable macro? How?

Oh, never mind. I forgot that __attribute__((deprecated(…))) supports a replacement but __attribute__((unavailable(…))) does not. (Arguably we could use __attribute__((available(*, unavailable, renamed="…"))) for this, but that can be tested out later.

jrose-apple · 2018-07-25T16:39:54Z

test/PrintAsObjC/availability.swift

+// CHECK-NEXT: - (nonnull instancetype)initWithNewZ:(NSInteger)z OBJC_DESIGNATED_INITIALIZER;
+// CHECK-NEXT: @end
+
+// CHECK-LABEL: SWIFT_AVAILABILITY(macos,deprecated=0.0.1,message="'DeprecatedAvailability' has been renamed to 'SWTReplacementAvailable'")


Another case where we ought to support renamed="" but don't. (Not a problem for this PR.)

jrose-apple · 2018-07-26T04:09:52Z

@swift-ci Please test

swift-ci · 2018-07-26T04:11:47Z

Build failed
Swift Test Linux Platform
Git Sha - 0e229a801e50b55aa60d00a017b761d2a5addc74

swift-ci · 2018-07-26T04:13:22Z

Build failed
Swift Test OS X Platform
Git Sha - 0e229a801e50b55aa60d00a017b761d2a5addc74

pitiphong-p · 2018-07-26T04:31:50Z

@jrose-apple Thank you. I'll add the test cases for the unavailable in the afternoon Thailand time and will ask Swift-CI to run the test. Hope it'll be ready to merge within this week :D

…orm unavailable PrintAsObjC

pitiphong-p · 2018-07-26T17:31:02Z

@jrose-apple I added the remaining platform deprecation, unavailable and platform unavailable test cases. Could you have a look again and ask the @swift-ci to run a test

jrose-apple · 2018-07-27T03:02:02Z

@swift-ci Please test

swift-ci · 2018-07-27T03:03:52Z

Build failed
Swift Test Linux Platform
Git Sha - b3a0a5e

swift-ci · 2018-07-27T03:03:52Z

Build failed
Swift Test OS X Platform
Git Sha - b3a0a5e

jrose-apple · 2018-07-27T03:06:57Z

test/PrintAsObjC/availability.swift

    @objc public func methodWithoutCustomObjCName(value: Int) -> Int { return -1 }

+    @available(*, deprecated, renamed: "methodWithoutCustomObjCName(value:)")
+    @objc public func deprecatedMethodRenamedToMethodWithouCustomObjCName(value: Int) -> Int { return -1 }


Not that it's super important, but for this family you wrote "Withou" instead of "Without".

@jrose-apple Should I fix it here or can leave it for the next PR?

I think it's okay to leave it. Let's get this merged!

jrose-apple · 2018-07-30T18:36:14Z

Thanks once again for working on this! Looking forward to more patches from you in the future.

pitiphong-p · 2018-07-31T05:59:19Z

@jrose-apple OMG OMG OMG Thank you for helping me from the start. I'm glad to have a chance to work on this and on Swift open source project. I'm looking forward to contribute to Swift in the future too :D

pitiphong-p changed the title ~~[WIP] Use the custom @objc name in the available attribute when generates ObjC headers~~ Use the custom @objc name in the available attribute when generates ObjC headers Jul 19, 2018

jrose-apple self-assigned this Jul 19, 2018

varokas reviewed Jul 19, 2018

View reviewed changes

jrose-apple reviewed Jul 21, 2018

View reviewed changes

pitiphong-p added 13 commits July 21, 2018 17:40

Use the custom @objc name in the Availablity attributes when generate…

d080189

…s the ObjC header

Added: More test cases for the PrintAsObjC availability

5b247c7

Reserve the white spaces

12d0024

Use the custom @objc name in the Availablity attributes on classes an…

7da1f30

…d protocols when generates the ObjC header

Added: Test for the class and protocol cases for the PrintAsObjC avai…

dc3b780

…lability

Rename findRenamedDecl method to printRenameForDecl

7a9b890

Pass the UnqualifiedLookup::Flags::TypeLookup option when doing the…

4661166

… Type lookup

Ditch the unneccessary Optional type on renamedFuncDecl variable

16ccb19

Added: Test case for the renamed to one of the overload methods case

b6cd6bf

Added: Test case for the renamed to one of the initializer case

8837ca0

Added: Test case for the case where the renamed method is class metho…

59b39bc

…d not instance method

Added: Test case for the case where the renamed method is class metho…

db0eafe

…d not instance method and vice versa

Fix the wrong checking condition on the case where both of them shoul…

045fb7c

…d be instance members or otherwise

pitiphong-p force-pushed the objc-rename-attribute branch from 0824aa7 to 045fb7c Compare July 21, 2018 17:10

jrose-apple reviewed Jul 24, 2018

View reviewed changes

pitiphong-p added 3 commits July 24, 2018 08:49

Mark the renamedFuncDecl variable as const

18ac6f9

Change the type of D parameter to ValueDecl

f852f3f

Use the CHECK-SAME where appropriate and update some ObjC names

2067935

Added: renaming to a method that does not have a custom Objective-C n…

1cdbe33

…ame test case

jrose-apple reviewed Jul 25, 2018

View reviewed changes

pitiphong-p added 2 commits July 25, 2018 14:07

Rename renamedFuncDecl to renamedDecl

a2c877a

Change from early exit to assert instead

e006d70

pitiphong-p added 2 commits July 25, 2018 22:59

Added: New test file for the renaming to the Objective-C functions

65dc151

Add the deprecation messages in the renamed-to Objective-C name test …

b3a0a5e

…cases

jrose-apple reviewed Jul 25, 2018

View reviewed changes

Added: Test cases for the platform deprecation, unavailable and platf…

0a29962

…orm unavailable PrintAsObjC

jrose-apple approved these changes Jul 27, 2018

View reviewed changes

jrose-apple merged commit 2d88bb6 into apple:master Jul 30, 2018

pitiphong-p deleted the objc-rename-attribute branch July 31, 2018 05:58

pitiphong-p mentioned this pull request Oct 1, 2018

Improve the behavior of the custom @objc name in the available attribute when generates ObjC headers #19645

Merged

pitiphong-p mentioned this pull request Jan 21, 2019

Cherry-Picking of Improve the behavior of the custom @objc name in the available attribute when generates ObjC headers to Swift 5.0 #22025

Closed

pitiphong-p mentioned this pull request Jul 19, 2018

[SR-8231] Compiler use the Swift name instead of the proper Objective-C name when renaming the usage of deprecated Swift API in Objective-C codebase #50763

Closed

		@@ -82,7 +106,7 @@

		@objc class Availability {
		@objc func alwaysAvailable() {}

Use the custom @objc name in the available attribute when generates ObjC headers #18081

Use the custom @objc name in the available attribute when generates ObjC headers #18081

Conversation

pitiphong-p commented Jul 19, 2018

pitiphong-p commented Jul 19, 2018

jrose-apple commented Jul 19, 2018

varokas Jul 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitiphong-p commented Jul 19, 2018

jrose-apple left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitiphong-p commented Jul 21, 2018 • edited

jrose-apple commented Jul 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrose-apple commented Jul 24, 2018

pitiphong-p commented Jul 24, 2018 • edited

Command Output (stderr):

pitiphong-p commented Jul 24, 2018

jrose-apple commented Jul 25, 2018

jrose-apple left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrose-apple commented Jul 25, 2018

pitiphong-p commented Jul 25, 2018

pitiphong-p commented Jul 25, 2018

jrose-apple left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrose-apple commented Jul 26, 2018

swift-ci commented Jul 26, 2018

swift-ci commented Jul 26, 2018

pitiphong-p commented Jul 26, 2018

pitiphong-p commented Jul 26, 2018

jrose-apple commented Jul 27, 2018

swift-ci commented Jul 27, 2018

swift-ci commented Jul 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrose-apple commented Jul 30, 2018

pitiphong-p commented Jul 31, 2018

varokas Jul 19, 2018 •

edited

pitiphong-p commented Jul 21, 2018 •

edited

pitiphong-p commented Jul 24, 2018 •

edited