Unwind the Bogus Delayed Categories Hack

Delayed categories only existed to keep
ClangImporter/objc_redeclared_properties passing.  But this test
appears to exhibit incorrect behavior as written.

The clang importer implements limited behavior for overwriting
variable declarations. If that overwrite occurs somewhere else in the
class hierarchy, the declaration is rewritten as a Swift override.  The
remaining case is categories, which enable all sorts of awful
redeclaration behaviors.  When a bridging header declares a category
that tries to stomp the API surface of another category, we need to
import that category first, and we would import that category first - by
accident.  Re-entrancy does have its upsides.

In order to keep the tests passing post re-entrant lookup, we stubbed in a hack that delayed
categories in the bridging header so they were installed last, which
meant the variable merging logic would mostly decline to import those
properties.  But the rest of the world expects the opposite: Bridging
header contents are more sacred than the rest of the SDK and must be
installed *first* in order for a number of nightmarish header hacks to
go through.

Unwind the old, incorrect emergent behavior and re-establish the
correct behavior as the way the world ought to work.

Resolves rdar://58493356, rdar://58493357, rdar://58493362, rdar://58493370

Author: CodaFi

lib/ClangImporter/ClangImporter.cpp

@@ -2974,32 +2974,8 @@ void ClangImporter::loadExtensions(NominalTypeDecl *nominal,
     SmallVector<clang::NamedDecl *, 4> DelayedCategories;
 
     // Simply importing the categories adds them to the list of extensions.
-    for (auto I = objcClass->visible_categories_begin(),
-           E = objcClass->visible_categories_end();
-         I != E; ++I) {
-      // Delay installing categories that don't have an owning module.
-      if (!I->hasOwningModule()) {
-        DelayedCategories.push_back(*I);
-        continue;
-      }
-
-      Impl.importDeclReal(*I, Impl.CurrentVersion);
-    }
-
-    // Install all the delayed categories.
-    //
-    // The very notion of a delayed category is a result of an emergent behavior
-    // of the visible categories list and the order we import modules. The list
-    // appears in deserialization order rather than some "source order", so it's
-    // possible for, say, a bridging header to import a module that defines an
-    // interface and some categories, but for the categories in the bridging
-    // header to appear *before* the categories in the module - the imported
-    // module will be deserialized on demand. We take it on faith that if there
-    // is no owning module for a given category, that it was created in such a
-    // way, and thus we install it last to try to emulate what we want
-    // "source order" to mean.
-    for (const auto *DelayedCat : DelayedCategories) {
-      Impl.importDeclReal(DelayedCat, Impl.CurrentVersion);
+    for (const auto *Cat : objcClass->visible_categories()) {
+      Impl.importDeclReal(Cat, Impl.CurrentVersion);
     }
   }
 

test/ClangImporter/objc_redeclared_properties_incompatible.swift

@@ -75,27 +75,33 @@ func testCategoryWithInitializer() {
   let obj = PropertiesInitCategory()
 
   let _: Int = obj.nullabilityChange
-  // CHECK: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PUBLIC: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PRIVATE: [[@LINE-2]]:20: error: cannot convert value of type 'Base?' to specified type 'Int'
 
   let _: Int = obj.missingGenerics
-  // CHECK: [[@LINE-1]]:20: error: cannot convert value of type 'GenericClass<Base>' to specified type 'Int'
+  // CHECK-PUBLIC: [[@LINE-1]]:20: error: cannot convert value of type 'GenericClass<Base>' to specified type 'Int'
+  // CHECK-PRIVATE: [[@LINE-2]]:20: error: cannot convert value of type 'GenericClass<AnyObject>' to specified type 'Int'
 
   let _: Int = obj.typeChange
-  // CHECK: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PUBLIC: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PRIVATE: [[@LINE-2]]:20: error: cannot convert value of type 'PrivateSubclass' to specified type 'Int'
 
   obj.readwriteChange = Base() // CHECK-PRIVATE-NOT: [[@LINE]]:{{.+}}: error
   // CHECK-PUBLIC: [[@LINE-1]]:7: error: cannot assign to property: 'readwriteChange' is a get-only property
 }
 
 func testCategoryWithoutInitializer(obj: PropertiesNoInitCategory) {
   let _: Int = obj.nullabilityChange
-  // CHECK: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PUBLIC: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PRIVATE: [[@LINE-2]]:20: error: cannot convert value of type 'Base?' to specified type 'Int'
 
   let _: Int = obj.missingGenerics
-  // CHECK: [[@LINE-1]]:20: error: cannot convert value of type 'GenericClass<Base>' to specified type 'Int'
+  // CHECK-PUBLIC: [[@LINE-1]]:20: error: cannot convert value of type 'GenericClass<Base>' to specified type 'Int'
+  // CHECK-PRIVATE: [[@LINE-2]]:20: error: cannot convert value of type 'GenericClass<AnyObject>' to specified type 'Int'
 
   let _: Int = obj.typeChange
-  // CHECK: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PUBLIC: [[@LINE-1]]:20: error: cannot convert value of type 'Base' to specified type 'Int'
+  // CHECK-PRIVATE: [[@LINE-2]]:20: error: cannot convert value of type 'PrivateSubclass' to specified type 'Int'
 
   obj.readwriteChange = Base() // CHECK-PRIVATE-NOT: [[@LINE]]:{{.+}}: error
   // CHECK-PUBLIC: [[@LINE-1]]:7: error: cannot assign to property: 'readwriteChange' is a get-only property