Merge pull request #6990 from jrose-apple/available-is-better-than-unavailable

jrose-apple · web-flow · commit 6fed3d1edf9b · 2017-01-26T11:52:36.000-08:00
[ClangImporter] Prefer available enum elements over unavailable ones.
diff --git a/lib/ClangImporter/ClangAdapter.cpp b/lib/ClangImporter/ClangAdapter.cpp
@@ -671,15 +671,9 @@ bool importer::isUnavailableInSwift(
   if (enableObjCInterop && isObjCId(decl))
     return true;
 
-  // FIXME: Somewhat duplicated from importAttributes(), but this is a
-  // more direct path.
-  if (decl->getAvailability() == clang::AR_Unavailable)
+  if (decl->isUnavailable())
     return true;
 
-  // Apply the deprecated-as-unavailable filter.
-  if (!platformAvailability.deprecatedAsUnavailableFilter)
-    return false;
-
   for (auto *attr : decl->specific_attrs<clang::AvailabilityAttr>()) {
     if (attr->getPlatform()->getName() == "swift")
       return true;
@@ -689,12 +683,15 @@ bool importer::isUnavailableInSwift(
       continue;
     }
 
-    clang::VersionTuple version = attr->getDeprecated();
-    if (version.empty())
-      continue;
-    if (platformAvailability.deprecatedAsUnavailableFilter(version.getMajor(),
-                                                           version.getMinor()))
-      return true;
+    if (platformAvailability.deprecatedAsUnavailableFilter) {
+      clang::VersionTuple version = attr->getDeprecated();
+      if (version.empty())
+        continue;
+      if (platformAvailability.deprecatedAsUnavailableFilter(
+            version.getMajor(), version.getMinor())) {
+        return true;
+      }
+    }
   }
 
   return false;
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -1800,6 +1800,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
@@ -2481,36 +2501,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;
 
@@ -2532,7 +2603,7 @@ namespace {
           if (errorWrapper) {
             auto enumeratorValue = cast<ValueDecl>(enumeratorDecl);
             auto alias = importEnumCaseAlias(enumeratorValue->getName(),
-                                             *ec,
+                                             constant,
                                              enumeratorValue,
                                              decl,
                                              enumeratorContext,
@@ -4751,14 +4822,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;
@@ -4776,7 +4839,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/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -1405,10 +1405,11 @@ const AvailableAttr *TypeChecker::getDeprecated(const Decl *D) {
 
 /// Returns true if some declaration lexically enclosing the reference
 /// matches the passed in predicate and false otherwise.
-static bool someEnclosingDeclMatches(SourceRange ReferenceRange,
-                                     const DeclContext *ReferenceDC,
-                                     TypeChecker &TC,
-                                     std::function<bool(const Decl *)> Pred) {
+static bool
+someEnclosingDeclMatches(SourceRange ReferenceRange,
+                         const DeclContext *ReferenceDC,
+                         TypeChecker &TC,
+                         llvm::function_ref<bool(const Decl *)> Pred) {
   ASTContext &Ctx = TC.Context;
 
   // Climb the DeclContext hierarchy to see if any of the containing
@@ -1500,7 +1501,7 @@ bool TypeChecker::isInsideUnavailableDeclaration(
 
 bool TypeChecker::isInsideDeprecatedDeclaration(SourceRange ReferenceRange,
                                                 const DeclContext *ReferenceDC){
-  std::function<bool(const Decl *)> IsDeprecated = [](const Decl *D) {
+  auto IsDeprecated = [](const Decl *D) {
     return D->getAttrs().getDeprecated(D->getASTContext());
   };
 
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
 };
