[ClangImporter] Prefer available enum elements over unavailable ones.

jrose-apple · jrose-apple · commit 3411fc380e2e · 2017-01-23T15:34:20.000-08:00
...and avoid making aliases from one unavailable declaration to another.
If it's unavailable, we can just import it as a normal case and not
worry about it. This fixes an issue where Sema would try to diagnose
the body of an "alias" for referring to unavailable declarations.

(Background: enum cases in Swift have to have unique values, so we
import any duplicate values as static properties. Pattern matching
logic has a hack to recognize these particular static properties as
being "case-like".)

This commit also sinks enum element uniqueness checking into importing
the enum, instead of keeping a global map we never consult again. This
should save a small bit of memory.

rdar://problem/30025723
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -1792,6 +1792,26 @@ applyPropertyOwnership(VarDecl *prop,
 }
 
 namespace {
+  /// Customized llvm::DenseMapInfo for storing borrowed APSInts.
+  struct APSIntRefDenseMapInfo {
+    static inline const llvm::APSInt *getEmptyKey() {
+      return llvm::DenseMapInfo<const llvm::APSInt *>::getEmptyKey();
+    }
+    static inline const llvm::APSInt *getTombstoneKey() {
+      return llvm::DenseMapInfo<const llvm::APSInt *>::getTombstoneKey();
+    }
+    static unsigned getHashValue(const llvm::APSInt *ptrVal) {
+      assert(ptrVal != getEmptyKey() && ptrVal != getTombstoneKey());
+      return llvm::hash_value(*ptrVal);
+    }
+    static bool isEqual(const llvm::APSInt *lhs, const llvm::APSInt *rhs) {
+      if (lhs == rhs) return true;
+      if (lhs == getEmptyKey() || rhs == getEmptyKey()) return false;
+      if (lhs == getTombstoneKey() || rhs == getTombstoneKey()) return false;
+      return *lhs == *rhs;
+    }
+  };
+
   /// \brief Convert Clang declarations into the corresponding Swift
   /// declarations.
   class SwiftDeclConverter
@@ -2473,36 +2493,87 @@ namespace {
         addEnumeratorsAsMembers = true;
         break;
       }
-      
-      for (auto ec = decl->enumerator_begin(), ecEnd = decl->enumerator_end();
-           ec != ecEnd; ++ec) {
+
+      llvm::SmallDenseMap<const llvm::APSInt *,
+                          PointerUnion<const clang::EnumConstantDecl *,
+                                       EnumElementDecl *>, 8,
+                          APSIntRefDenseMapInfo> canonicalEnumConstants;
+
+      if (enumKind == EnumKind::Enum) {
+        for (auto constant : decl->enumerators()) {
+          if (Impl.isUnavailableInSwift(constant))
+            continue;
+          canonicalEnumConstants.insert({&constant->getInitVal(), constant});
+        }
+      }
+
+      for (auto constant : decl->enumerators()) {
         Decl *enumeratorDecl;
         Decl *swift2EnumeratorDecl = nullptr;
         switch (enumKind) {
         case EnumKind::Constants:
         case EnumKind::Unknown:
-          enumeratorDecl = Impl.importDecl(*ec, getActiveSwiftVersion());
+          enumeratorDecl = Impl.importDecl(constant, getActiveSwiftVersion());
           swift2EnumeratorDecl =
-              Impl.importDecl(*ec, ImportNameVersion::Swift2);
+              Impl.importDecl(constant, ImportNameVersion::Swift2);
           break;
         case EnumKind::Options:
           enumeratorDecl =
               SwiftDeclConverter(Impl, getActiveSwiftVersion())
-                  .importOptionConstant(*ec, decl, enumeratorContext);
+                  .importOptionConstant(constant, decl, enumeratorContext);
           swift2EnumeratorDecl =
               SwiftDeclConverter(Impl, ImportNameVersion::Swift2)
-                  .importOptionConstant(*ec, decl, enumeratorContext);
+                  .importOptionConstant(constant, decl, enumeratorContext);
           break;
-        case EnumKind::Enum:
-          enumeratorDecl =
-              SwiftDeclConverter(Impl, getActiveSwiftVersion())
-                  .importEnumCase(*ec, decl, cast<EnumDecl>(enumeratorContext));
+        case EnumKind::Enum: {
+          auto canonicalCaseIter =
+            canonicalEnumConstants.find(&constant->getInitVal());
+
+          if (canonicalCaseIter == canonicalEnumConstants.end()) {
+            // Unavailable declarations get no special treatment.
+            enumeratorDecl =
+                SwiftDeclConverter(Impl, getActiveSwiftVersion())
+                    .importEnumCase(constant, decl,
+                                    cast<EnumDecl>(enumeratorContext));
+          } else {
+            const clang::EnumConstantDecl *unimported =
+                canonicalCaseIter->
+                  second.dyn_cast<const clang::EnumConstantDecl *>();
+
+            // Import the canonical enumerator for this case first.
+            if (unimported) {
+              enumeratorDecl = SwiftDeclConverter(Impl, getActiveSwiftVersion())
+                  .importEnumCase(unimported, decl,
+                                  cast<EnumDecl>(enumeratorContext));
+              if (enumeratorDecl) {
+                canonicalCaseIter->getSecond() =
+                    cast<EnumElementDecl>(enumeratorDecl);
+              }
+            } else {
+              enumeratorDecl =
+                  canonicalCaseIter->second.get<EnumElementDecl *>();
+            }
+
+            if (unimported != constant && enumeratorDecl) {
+              ImportedName importedName =
+                  Impl.importFullName(constant, getActiveSwiftVersion());
+              Identifier name = importedName.getDeclName().getBaseName();
+              if (!name.empty()) {
+                auto original = cast<ValueDecl>(enumeratorDecl);
+                enumeratorDecl = importEnumCaseAlias(name, constant, original,
+                                                     decl, enumeratorContext);
+              }
+            }
+          }
+
           swift2EnumeratorDecl =
               SwiftDeclConverter(Impl, ImportNameVersion::Swift2)
-                  .importEnumCase(*ec, decl, cast<EnumDecl>(enumeratorContext),
+                  .importEnumCase(constant, decl,
+                                  cast<EnumDecl>(enumeratorContext),
                                   enumeratorDecl);
           break;
         }
+        }
         if (!enumeratorDecl)
           continue;
 
@@ -2524,7 +2595,7 @@ namespace {
           if (errorWrapper) {
             auto enumeratorValue = cast<ValueDecl>(enumeratorDecl);
             auto alias = importEnumCaseAlias(enumeratorValue->getName(),
-                                             *ec,
+                                             constant,
                                              enumeratorValue,
                                              decl,
                                              enumeratorContext,
@@ -4743,14 +4814,6 @@ Decl *SwiftDeclConverter::importEnumCase(const clang::EnumConstantDecl *decl,
   bool negative = false;
   llvm::APSInt rawValue = decl->getInitVal();
 
-  // Did we already import an enum constant for this enum with the
-  // same value? If so, import it as a standalone constant.
-  auto insertResult =
-      Impl.EnumConstantValues.insert({{clangEnum, rawValue}, nullptr});
-  if (!insertResult.second)
-    return importEnumCaseAlias(name, decl, insertResult.first->second,
-                               clangEnum, theEnum);
-
   if (clangEnum->getIntegerType()->isSignedIntegerOrEnumerationType() &&
       rawValue.slt(0)) {
     rawValue = -rawValue;
@@ -4768,7 +4831,6 @@ Decl *SwiftDeclConverter::importEnumCase(const clang::EnumConstantDecl *decl,
   auto element = Impl.createDeclWithClangNode<EnumElementDecl>(
       decl, Accessibility::Public, SourceLoc(), name, TypeLoc(), SourceLoc(),
       rawValueExpr, theEnum);
-  insertResult.first->second = element;
 
   // Give the enum element the appropriate type.
   element->computeType();
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
@@ -410,37 +410,12 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   LookupTableMap &getLookupTables() { return LookupTables; }
 
 private:
-  class EnumConstantDenseMapInfo {
-  public:
-    using PairTy = std::pair<const clang::EnumDecl *, llvm::APSInt>;
-    using PointerInfo = llvm::DenseMapInfo<const clang::EnumDecl *>;
-    static inline PairTy getEmptyKey() {
-      return {PointerInfo::getEmptyKey(), llvm::APSInt(/*bitwidth=*/1)};
-    }
-    static inline PairTy getTombstoneKey() {
-      return {PointerInfo::getTombstoneKey(), llvm::APSInt(/*bitwidth=*/1)};
-    }
-    static unsigned getHashValue(const PairTy &pair) {
-      return llvm::combineHashValue(PointerInfo::getHashValue(pair.first),
-                                    llvm::hash_value(pair.second));
-    }
-    static bool isEqual(const PairTy &lhs, const PairTy &rhs) {
-      return lhs == rhs;
-    }
-  };
-
   /// A mapping from imported declarations to their "alternate" declarations,
   /// for cases where a single Clang declaration is imported to two
   /// different Swift declarations.
   llvm::DenseMap<Decl *, TinyPtrVector<ValueDecl *>> AlternateDecls;
 
 public:
-  /// \brief Keep track of enum constant values that have been imported.
-  llvm::DenseMap<std::pair<const clang::EnumDecl *, llvm::APSInt>,
-                 EnumElementDecl *,
-                 EnumConstantDenseMapInfo>
-    EnumConstantValues;
-
   /// \brief Keep track of initializer declarations that correspond to
   /// imported methods.
   llvm::DenseMap<
diff --git a/test/ClangImporter/enum.swift b/test/ClangImporter/enum.swift
@@ -1,9 +1,14 @@
 // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-sil %s -verify
+// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck %s 2>&1 | %FileCheck %s
 // -- Check that we can successfully round-trip.
 // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -D IRGEN -emit-ir %s >/dev/null
 
 // REQUIRES: objc_interop
 
+// At one point we diagnosed enum case aliases that referred to unavailable 
+// cases in their (synthesized) implementation.
+// CHECK-NOT: unknown
+
 import Foundation
 import user_objc
 
@@ -106,6 +111,15 @@ extension NSAliasesEnum {
   }
 }
 
+#if !IRGEN
+_ = NSUnavailableAliasesEnum.originalAU
+_ = NSUnavailableAliasesEnum.aliasAU // expected-error {{'aliasAU' is unavailable}}
+_ = NSUnavailableAliasesEnum.originalUA // expected-error {{'originalUA' is unavailable}}
+_ = NSUnavailableAliasesEnum.aliasUA
+_ = NSUnavailableAliasesEnum.originalUU // expected-error {{'originalUU' is unavailable}}
+_ = NSUnavailableAliasesEnum.aliasUU // expected-error {{'aliasUU' is unavailable}}
+#endif
+
 // Test NS_SWIFT_NAME:
 _ = XMLNode.Kind.DTDKind == .invalid
 
diff --git a/test/Inputs/clang-importer-sdk/usr/include/Foundation.h b/test/Inputs/clang-importer-sdk/usr/include/Foundation.h
@@ -381,6 +381,15 @@ typedef NS_ENUM(unsigned char, NSAliasesEnum) {
   NSAliasesDifferentValue = 2
 };
 
+typedef NS_ENUM(unsigned char, NSUnavailableAliasesEnum) {
+  NSUnavailableAliasesOriginalAU = 0,
+  NSUnavailableAliasesAliasAU __attribute__((unavailable)) = 0,
+  NSUnavailableAliasesOriginalUA __attribute__((unavailable)) = 1,
+  NSUnavailableAliasesAliasUA = 1,
+  NSUnavailableAliasesOriginalUU __attribute__((unavailable)) = 2,
+  NSUnavailableAliasesAliasUU __attribute__((unavailable)) = 2,
+};
+
 NS_ENUM(NSInteger, NSMalformedEnumMissingTypedef) {
   NSMalformedEnumMissingTypedefValue
 };
