[cxx-interop] Check for NS_OPTIONS macro in findOptionSetEnum()

importer::findOptionSetEnum() uses some fragile heuristics to determine
whether a typedef is involved in the construction of a CF_OPTIONS or
NS_OPTIONS type. This patch adds an explicit check that the typedef is
expanded from either of those macros, to prevent, e.g., an unavailable
NS_ENUM, from being mistakenly recognized as an NS_OPTIONS.

Note that doing this is still kind of fragile, and prevents users from
building {NS,CF}_OPTIONS with their own macros. The right thing to do is
probably specifically look for the flag_enum attribute, but that is not
currently what we're doing for reasons whose discovery is left as
an exercise to the future git archaeologist.

This patch also removes (part of) a test case that builds
a CF_OPTIONS-like type with the "SOME_OPTIONS" macro, which is no longer
supported as of this patch.

rdar://150399978

Author: j-hui

lib/ClangImporter/ImportEnumInfo.cpp

@@ -255,6 +255,11 @@ ImportedType importer::findOptionSetEnum(clang::QualType type,
     // then this definitely isn't used for {CF,NS}_OPTIONS.
     return ImportedType();
 
+  if (Impl.SwiftContext.LangOpts.EnableCXXInterop &&
+      !isCFOptionsMacro(typedefType->getDecl(), Impl.getClangPreprocessor())) {
+    return ImportedType();
+  }
+
   auto clangEnum = findAnonymousEnumForTypedef(Impl.SwiftContext, typedefType);
   if (!clangEnum)
     return ImportedType();

lib/ClangImporter/ImportEnumInfo.h

@@ -19,6 +19,8 @@
 
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/Decl.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/Sema.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/DenseMap.h"
 
@@ -164,14 +166,18 @@ StringRef getCommonPluralPrefix(StringRef singular, StringRef plural);
 /// an elaborated type, an unwrapped type is returned.
 const clang::Type *getUnderlyingType(const clang::EnumDecl *decl);
 
-inline bool isCFOptionsMacro(StringRef macroName) {
-  return llvm::StringSwitch<bool>(macroName)
+inline bool isCFOptionsMacro(const clang::NamedDecl *decl,
+                             clang::Preprocessor &preprocessor) {
+  auto loc = decl->getEndLoc();
+  if (!loc.isMacroID())
+    return false;
+  return llvm::StringSwitch<bool>(preprocessor.getImmediateMacroName(loc))
       .Case("CF_OPTIONS", true)
       .Case("NS_OPTIONS", true)
       .Default(false);
 }
 
-}
-}
+} // namespace importer
+} // namespace swift
 
 #endif // SWIFT_CLANG_IMPORT_ENUM_H

lib/ClangImporter/ImportName.cpp

@@ -18,6 +18,7 @@
 #include "CFTypeInfo.h"
 #include "ClangClassTemplateNamePrinter.h"
 #include "ClangDiagnosticConsumer.h"
+#include "ImportEnumInfo.h"
 #include "ImporterImpl.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/ClangSwiftTypeCorrespondence.h"
@@ -1877,15 +1878,9 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
   // imported into Swift to avoid having two types with the same name, which
   // cause subtle name lookup issues.
   if (swiftCtx.LangOpts.EnableCXXInterop &&
-      isUnavailableInSwift(D, nullptr, true)) {
-    auto loc = D->getEndLoc();
-    if (loc.isMacroID()) {
-      StringRef macroName =
-          clangSema.getPreprocessor().getImmediateMacroName(loc);
-      if (isCFOptionsMacro(macroName))
-        return ImportedName();
-    }
-  }
+      isUnavailableInSwift(D, nullptr, true) &&
+      isCFOptionsMacro(D, clangSema.getPreprocessor()))
+    return ImportedName();
 
   /// Whether the result is a function name.
   bool isFunction = false;

lib/ClangImporter/ImportType.cpp

@@ -15,6 +15,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "CFTypeInfo.h"
+#include "ImportEnumInfo.h"
 #include "ImporterImpl.h"
 #include "SwiftDeclSynthesizer.h"
 #include "swift/ABI/MetadataValues.h"
@@ -2909,13 +2910,8 @@ ArgumentAttrs ClangImporter::Implementation::inferDefaultArgument(
           return argumentAttrs;
         }
       }
-      auto loc = typedefDecl->getEndLoc();
-      if (loc.isMacroID()) {
-        StringRef macroName =
-            nameImporter.getClangPreprocessor().getImmediateMacroName(loc);
-        if (isCFOptionsMacro(macroName))
-          return argumentAttrs;
-      }
+      if (isCFOptionsMacro(typedefDecl, nameImporter.getClangPreprocessor()))
+        return argumentAttrs;
     }
   }
 

test/Interop/Cxx/enum/Inputs/anonymous-with-swift-name.h

@@ -1,38 +1,28 @@
 #ifndef TEST_INTEROP_CXX_ENUM_INPUTS_ANONYMOUS_WITH_SWIFT_NAME_H
 #define TEST_INTEROP_CXX_ENUM_INPUTS_ANONYMOUS_WITH_SWIFT_NAME_H
 
-#define SOME_OPTIONS(_type, _name) __attribute__((availability(swift, unavailable))) _type _name; enum __attribute__((flag_enum,enum_extensibility(open))) : _name
 #define CF_OPTIONS(_type, _name) __attribute__((availability(swift, unavailable))) _type _name; enum : _name
 
-typedef SOME_OPTIONS(unsigned, SOColorMask) {
-    kSOColorMaskRed = (1 << 1),
-    kSOColorMaskGreen = (1 << 2),
-    kSOColorMaskBlue = (1 << 3),
-    kSOColorMaskAll = ~0U
-};
-
-
 typedef CF_OPTIONS(unsigned, CFColorMask) {
   kCFColorMaskRed = (1 << 1),
   kCFColorMaskGreen = (1 << 2),
   kCFColorMaskBlue = (1 << 3),
   kCFColorMaskAll = ~0U
 };
 
-inline SOColorMask useSOColorMask(SOColorMask mask) { return mask; }
 inline CFColorMask useCFColorMask(CFColorMask mask) { return mask; }
 
 struct ParentStruct { };
 
 inline CFColorMask renameCFColorMask(ParentStruct parent)
     __attribute__((swift_name("ParentStruct.childFn(self:)")))
-{ return kSOColorMaskRed; }
+{ return kCFColorMaskRed; }
 
 inline CFColorMask getCFColorMask(ParentStruct parent)
     __attribute__((swift_name("getter:ParentStruct.colorProp(self:)")))
-{ return kSOColorMaskRed; }
+{ return kCFColorMaskRed; }
 
-inline void getCFColorMask(ParentStruct parent, CFColorMask newValue)
+inline void setCFColorMask(ParentStruct parent, CFColorMask newValue)
     __attribute__((swift_name("setter:ParentStruct.colorProp(self:newValue:)")))
 { }
 
@@ -52,9 +42,8 @@ enum __attribute__((flag_enum,enum_extensibility(open))) : GlobalOldName {
 
 #if __OBJC__
 @interface ColorMaker
-- (void)makeColorWithOptions:(SOColorMask)opts;
 - (void)makeOtherColorWithInt:(int) x withOptions:(CFColorMask)opts;
 @end
 #endif // SWIFT_OBJC_INTEROP
 
-#endif // TEST_INTEROP_CXX_ENUM_INPUTS_ANONYMOUS_WITH_SWIFT_NAME_H
+#endif // TEST_INTEROP_CXX_ENUM_INPUTS_ANONYMOUS_WITH_SWIFT_NAME_H

test/Interop/Cxx/enum/anonymous-with-swift-name-module-interface.swift

@@ -1,32 +1,5 @@
 // RUN: %target-swift-ide-test -print-module -module-to-print=AnonymousWithSwiftName -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
 
-// CHECK: @available(*, unavailable, message: "Not available in Swift")
-// CHECK: typealias SOColorMask = UInt32
-
-// CHECK: struct SOColorMask : OptionSet, @unchecked Sendable {
-// CHECK:   init(rawValue: UInt32)
-// CHECK:   let rawValue: UInt32
-// CHECK:   typealias RawValue = UInt32
-// CHECK:   typealias Element = SOColorMask
-// CHECK:   typealias ArrayLiteralElement = SOColorMask
-
-// CHECK:   static var red: SOColorMask { get }
-// CHECK:   @available(swift, obsoleted: 3, renamed: "red")
-// CHECK:   static var Red: SOColorMask { get }
-
-// CHECK:   static var green: SOColorMask { get }
-// CHECK:   @available(swift, obsoleted: 3, renamed: "green")
-// CHECK:   static var Green: SOColorMask { get }
-
-// CHECK:   static var blue: SOColorMask { get }
-// CHECK:   @available(swift, obsoleted: 3, renamed: "blue")
-// CHECK:   static var Blue: SOColorMask { get }
-
-// CHECK:   static var all: SOColorMask { get }
-// CHECK:   @available(swift, obsoleted: 3, renamed: "all")
-// CHECK:   static var All: SOColorMask { get }
-// CHECK: }
-
 // CHECK-NOT: typealias CFColorMask = UInt32
 
 // CHECK: struct CFColorMask : OptionSet {
@@ -53,7 +26,6 @@
 // CHECK:   static var All: CFColorMask { get }
 // CHECK: }
 
-// CHECK: func useSOColorMask(_ mask: SOColorMask) -> SOColorMask
 // CHECK: func useCFColorMask(_ mask: CFColorMask) -> CFColorMask
 
 // Test rename with "swift_name" attr:

test/Interop/Cxx/enum/anonymous-with-swift-name-objc-module-interface.swift

@@ -3,12 +3,6 @@
 // REQUIRES: objc_interop
 
 // CHECK: class ColorMaker {
-// CHECK:   class func makeColor(withOptions opts: SOColorMask)
-// CHECK:   func makeColor(withOptions opts: SOColorMask)
-// CHECK:   @available(swift, obsoleted: 3, renamed: "makeColor(withOptions:)")
-// CHECK:   class func makeColorWithOptions(_ opts: SOColorMask)
-// CHECK:   @available(swift, obsoleted: 3, renamed: "makeColor(withOptions:)")
-// CHECK:   func makeColorWithOptions(_ opts: SOColorMask)
 // CHECK:   class func makeOtherColor(with x: Int32, withOptions opts: CFColorMask)
 // CHECK:   func makeOtherColor(with x: Int32, withOptions opts: CFColorMask)
 // CHECK:   @available(swift, obsoleted: 3, renamed: "makeOtherColor(with:withOptions:)")

test/Interop/Cxx/enum/anonymous-with-swift-name.swift

@@ -7,18 +7,6 @@ import StdlibUnittest
 
 var AnonymousEnumsTestSuite = TestSuite("Anonymous Enums With Swift Name")
 
-AnonymousEnumsTestSuite.test("SOME_OPTIONS") {
-  let red: SOColorMask = .red
-  let green = SOColorMask.green
-  let blue = .blue as SOColorMask
-  let all: SOColorMask = .all
-
-  expectEqual(red.rawValue, 2)
-  expectEqual(green.rawValue, 4)
-  expectEqual(blue.rawValue, 8)
-   expectEqual(all.rawValue, ~CUnsignedInt(0))
-}
-
 AnonymousEnumsTestSuite.test("CF_OPTIONS") {
   let red: CFColorMask = .red
   let green = CFColorMask.green
@@ -35,8 +23,8 @@ AnonymousEnumsTestSuite.test("Parameter types") {
   let red: CFColorMask = .red
   let green = CFColorMask.green
 
-  let blue = useSOColorMask(.blue)
-  let all = useSOColorMask(.all)
+  let blue = useCFColorMask(.blue)
+  let all = useCFColorMask(.all)
 
   expectEqual(red, useCFColorMask(.red))
   expectEqual(green, useCFColorMask(.green))