Remove funcsWithPointerBounds, add experimental langopt

Pass around hasSafePointer as a parameter flag instead, and only call
importBoundsAttributes when the imported function has a parameter or
return value with CountAttributedType.

Enables importing bounds safety pointers using
-enable-experimental-bounds-safety-interop rather than based on a
compile time define.

Author: hnrklssn

include/swift/Basic/LangOptions.h

@@ -325,6 +325,10 @@ namespace swift {
     /// disabled because it is not complete.
     bool EnableCXXInterop = false;
 
+    /// Enable C interop importing bounds safety and lifetime attributes to
+    /// generate Swift wrappers with safe pointer types (experimental).
+    bool EnableBoundsSafetyInterop = false;
+
     /// The C++ interoperability source compatibility version. Defaults
     /// to the Swift language version.
     version::Version cxxInteropCompatVersion;

include/swift/Option/Options.td

@@ -792,6 +792,11 @@ def enable_experimental_concise_pound_file : Flag<["-"],
   Flags<[FrontendOption, ModuleInterfaceOption]>,
   HelpText<"Enable experimental concise '#file' identifier">;
 
+def enable_experimental_bounds_safety_interop :
+  Flag<["-"], "enable-experimental-bounds-safety-interop">,
+  Flags<[NoDriverOption, FrontendOption, HelpHidden, ModuleInterfaceOption]>,
+  HelpText<"Enable experimental C bounds safety interop code generation and config directives">;
+
 def enable_experimental_cxx_interop :
   Flag<["-"], "enable-experimental-cxx-interop">,
   Flags<[NoDriverOption, FrontendOption, HelpHidden, ModuleInterfaceOption]>,

lib/ClangImporter/ClangImporter.cpp

@@ -576,9 +576,8 @@ void importer::getNormalInvocationArguments(
     }
   }
 
-#ifdef SWIFT_ENABLE_EXPERIMENTAL_POINTER_BOUNDS
-  invokationsArgStrs.push_back("-fexperimental-bounds-safety-attributes");
-#endif
+  if (LangOpts.EnableBoundsSafetyInterop)
+    invocationArgStrs.push_back("-fexperimental-bounds-safety-attributes");
 
   // Set C language options.
   if (triple.isOSDarwin()) {

lib/ClangImporter/ImportDecl.cpp

@@ -109,7 +109,8 @@ createFuncOrAccessor(ClangImporter::Implementation &impl, SourceLoc funcLoc,
                      std::optional<AccessorInfo> accessorInfo, DeclName name,
                      SourceLoc nameLoc, GenericParamList *genericParams,
                      ParameterList *bodyParams, Type resultTy, bool async,
-                     bool throws, DeclContext *dc, ClangNode clangNode) {
+                     bool throws, DeclContext *dc, ClangNode clangNode,
+                     bool hasSafePointer) {
   FuncDecl *decl;
   if (accessorInfo) {
     decl = AccessorDecl::create(
@@ -125,7 +126,9 @@ createFuncOrAccessor(ClangImporter::Implementation &impl, SourceLoc funcLoc,
                                     genericParams, dc, clangNode);
   }
   impl.importSwiftAttrAttributes(decl);
-  impl.importBoundsAttributes(decl);
+  if (hasSafePointer)
+    impl.importBoundsAttributes(decl);
+
   return decl;
 }
 
@@ -3254,7 +3257,8 @@ namespace {
       }
       return Impl.importFunctionParameterList(
           dc, decl, nonSelfParams, decl->isVariadic(), allowNSUIntegerAsInt,
-          argNames, genericParams, /*resultType=*/nullptr);
+          argNames, genericParams, /*resultType=*/nullptr,
+          /*hasSafePointerParam=*/nullptr);
     }
 
     Decl *
@@ -3635,6 +3639,7 @@ namespace {
 
       bool importFuncWithoutSignature =
           isa<clang::CXXMethodDecl>(decl) && Impl.importSymbolicCXXDecls;
+      bool hasSafePointer = false;
       if (!dc->isModuleScopeContext() && !isa<clang::CXXMethodDecl>(decl)) {
         // Handle initializers.
         if (name.getBaseName().isConstructor()) {
@@ -3731,7 +3736,7 @@ namespace {
           importedType = Impl.importFunctionParamsAndReturnType(
               dc, decl, {decl->param_begin(), decl->param_size()},
               decl->isVariadic(), isInSystemModule(dc), name, bodyParams,
-              templateParams);
+              templateParams, &hasSafePointer);
         }
 
         if (auto *mdecl = dyn_cast<clang::CXXMethodDecl>(decl)) {
@@ -3797,11 +3802,10 @@ namespace {
       } else {
         auto resultTy = importedType.getType();
 
-        FuncDecl *func =
-            createFuncOrAccessor(Impl, loc, accessorInfo, name,
-                                 nameLoc, genericParams, bodyParams, resultTy,
-                                 /*async=*/false, /*throws=*/false, dc,
-                                 clangNode);
+        FuncDecl *func = createFuncOrAccessor(
+            Impl, loc, accessorInfo, name, nameLoc, genericParams, bodyParams,
+            resultTy,
+            /*async=*/false, /*throws=*/false, dc, clangNode, hasSafePointer);
         result = func;
 
         if (!dc->isModuleScopeContext()) {
@@ -4840,12 +4844,13 @@ namespace {
         }
       }
 
-      auto result = createFuncOrAccessor(Impl,
-                                         /*funcLoc*/ SourceLoc(), accessorInfo,
-                                         importedName.getDeclName(),
-                                         /*nameLoc*/ SourceLoc(),
-                                         /*genericParams=*/nullptr, bodyParams,
-                                         resultTy, async, throws, dc, decl);
+      bool hasSafePointer = false; // currently only implemented for functions
+      auto result = createFuncOrAccessor(
+          Impl,
+          /*funcLoc*/ SourceLoc(), accessorInfo, importedName.getDeclName(),
+          /*nameLoc*/ SourceLoc(),
+          /*genericParams=*/nullptr, bodyParams, resultTy, async, throws, dc,
+          decl, hasSafePointer);
 
       result->setAccess(decl->isDirectMethod() ? AccessLevel::Public
                                                : getOverridableAccessLevel(dc));
@@ -6488,7 +6493,8 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer(
   } else {
     parameterList = Impl.importFunctionParameterList(
         dc, decl, {decl->param_begin(), decl->param_end()}, decl->isVariadic(),
-        allowNSUIntegerAsInt, argNames, /*genericParams=*/{}, /*resultType=*/nullptr);
+        allowNSUIntegerAsInt, argNames, /*genericParams=*/{},
+        /*resultType=*/nullptr, /*hasSafePointerParam=*/nullptr);
   }
   if (!parameterList)
     return nullptr;
@@ -8574,12 +8580,13 @@ class PointerParamInfoPrinter {
 
 void ClangImporter::Implementation::importBoundsAttributes(
     FuncDecl *MappedDecl) {
+  assert(SwiftContext.LangOpts.EnableBoundsSafetyInterop);
   auto ClangDecl =
       dyn_cast_or_null<clang::FunctionDecl>(MappedDecl->getClangDecl());
+  // any function with safe pointer imports should have a clang decl
+  assert(ClangDecl);
   if (!ClangDecl)
     return;
-  if (!funcsWithPointerBounds.count(ClangDecl))
-    return;
 
   llvm::SmallString<128> MacroString;
   {

lib/ClangImporter/ImportType.cpp

@@ -216,6 +216,7 @@ namespace {
     Bridgeability Bridging;
     const clang::FunctionType *CompletionHandlerType;
     std::optional<unsigned> CompletionHandlerErrorParamIndex;
+    bool isSafePointer = false;
 
   public:
     SwiftTypeConverter(ClangImporter::Implementation &impl,
@@ -238,6 +239,8 @@ namespace {
       return IR;
     }
 
+    bool hasSafePointer() { return isSafePointer; }
+
     ImportResult VisitType(const Type*) = delete;
 
     // TODO(https://github.com/apple/swift/issues/56206): Add support for dependent types.
@@ -412,13 +415,8 @@ namespace {
 
     ImportResult VisitCountAttributedType(
         const clang::CountAttributedType *type) {
-      // CountAttributedType is a clang type representing a pointer with
-      // a "counted_by" type attribute. For now, we don't import these
-      // into Swift.
-      // In the future we could do something more clever (such as trying to
-      // import as an Array where possible) or less clever (such as importing
-      // as the desugared, underlying pointer type).
-      return Type();
+      isSafePointer = true;
+      return Visit(type->desugar());
     }
 
     ImportResult VisitMemberPointerType(const clang::MemberPointerType *type) {
@@ -470,7 +468,9 @@ namespace {
       // without special hints.
       Type pointeeType = Impl.importTypeIgnoreIUO(
           pointeeQualType, ImportTypeKind::Value, addImportDiagnostic,
-          AllowNSUIntegerAsInt, Bridgeability::None, ImportTypeAttrs());
+          AllowNSUIntegerAsInt, Bridgeability::None, ImportTypeAttrs(),
+          OTK_ImplicitlyUnwrappedOptional, /*resugarNSErrorPointer=*/true,
+          &isSafePointer);
 
       // If this is imported as a reference type, ignore the innermost pointer.
       // (`T *` becomes `T`, but `T **` becomes `UnsafeMutablePointer<T>`.)
@@ -1689,7 +1689,8 @@ ImportedType ClangImporter::Implementation::importType(
     llvm::function_ref<void(Diagnostic &&)> addImportDiagnosticFn,
     bool allowNSUIntegerAsInt, Bridgeability bridging, ImportTypeAttrs attrs,
     OptionalTypeKind optionality, bool resugarNSErrorPointer,
-    std::optional<unsigned> completionHandlerErrorParamIndex) {
+    std::optional<unsigned> completionHandlerErrorParamIndex,
+    bool *isSafePointer) {
   if (type.isNull())
     return {Type(), false};
 
@@ -1751,6 +1752,8 @@ ImportedType ClangImporter::Implementation::importType(
       *this, addImportDiagnosticFn, allowNSUIntegerAsInt, bridging,
       completionHandlerType, completionHandlerErrorParamIndex);
   auto importResult = converter.Visit(type);
+  if (isSafePointer)
+    *isSafePointer |= converter.hasSafePointer();
 
   // Now fix up the type based on how we're concretely using it.
   auto adjustedType = adjustTypeForConcreteImport(
@@ -1764,13 +1767,13 @@ ImportedType ClangImporter::Implementation::importType(
 Type ClangImporter::Implementation::importTypeIgnoreIUO(
     clang::QualType type, ImportTypeKind importKind,
     llvm::function_ref<void(Diagnostic &&)> addImportDiagnosticFn,
-    bool allowNSUIntegerAsInt, Bridgeability bridging,
-    ImportTypeAttrs attrs, OptionalTypeKind optionality,
-    bool resugarNSErrorPointer) {
+    bool allowNSUIntegerAsInt, Bridgeability bridging, ImportTypeAttrs attrs,
+    OptionalTypeKind optionality, bool resugarNSErrorPointer,
+    bool *isSafePointer) {
 
-  auto importedType = importType(type, importKind, addImportDiagnosticFn,
-                                 allowNSUIntegerAsInt, bridging, attrs,
-                                 optionality, resugarNSErrorPointer);
+  auto importedType = importType(
+      type, importKind, addImportDiagnosticFn, allowNSUIntegerAsInt, bridging,
+      attrs, optionality, resugarNSErrorPointer, std::nullopt, isSafePointer);
 
   return importedType.getType();
 }
@@ -2155,7 +2158,7 @@ applyImportTypeAttrs(ImportTypeAttrs attrs, Type type,
 
 ImportedType ClangImporter::Implementation::importFunctionReturnType(
     DeclContext *dc, const clang::FunctionDecl *clangDecl,
-    bool allowNSUIntegerAsInt) {
+    bool allowNSUIntegerAsInt, bool *isSafePointer) {
 
   // Hardcode handling of certain result types for builtins.
   if (auto builtinID = clangDecl->getBuiltinID()) {
@@ -2264,13 +2267,13 @@ ImportedType ClangImporter::Implementation::importFunctionReturnType(
   }
 
   // Import the result type.
-  return importType(returnType,
-                    (isAuditedResult ? ImportTypeKind::AuditedResult
-                                     : ImportTypeKind::Result),
-                    ImportDiagnosticAdder(*this, clangDecl,
-                                          clangDecl->getLocation()),
-                    allowNSUIntegerAsInt, Bridgeability::Full,
-                    getImportTypeAttrs(clangDecl), OptionalityOfReturn);
+  return importType(
+      returnType,
+      (isAuditedResult ? ImportTypeKind::AuditedResult
+                       : ImportTypeKind::Result),
+      ImportDiagnosticAdder(*this, clangDecl, clangDecl->getLocation()),
+      allowNSUIntegerAsInt, Bridgeability::Full, getImportTypeAttrs(clangDecl),
+      OptionalityOfReturn, isSafePointer);
 }
 
 static Type
@@ -2305,7 +2308,7 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(
     DeclContext *dc, const clang::FunctionDecl *clangDecl,
     ArrayRef<const clang::ParmVarDecl *> params, bool isVariadic,
     bool isFromSystemModule, DeclName name, ParameterList *&parameterList,
-    ArrayRef<GenericTypeParamDecl *> genericParams) {
+    ArrayRef<GenericTypeParamDecl *> genericParams, bool *hasSafePointer) {
 
   bool allowNSUIntegerAsInt =
       shouldAllowNSUIntegerAsInt(isFromSystemModule, clangDecl);
@@ -2362,8 +2365,8 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(
     // If importedType is already initialized, it means we found the enum that
     // was supposed to be used (instead of the typedef type).
     if (!importedType) {
-      importedType =
-          importFunctionReturnType(dc, clangDecl, allowNSUIntegerAsInt);
+      importedType = importFunctionReturnType(
+          dc, clangDecl, allowNSUIntegerAsInt, hasSafePointer);
       if (!importedType) {
         addDiag(Diagnostic(diag::return_type_not_imported));
         return {Type(), false};
@@ -2373,9 +2376,9 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(
 
   Type swiftResultTy = importedType.getType();
   ArrayRef<Identifier> argNames = name.getArgumentNames();
-  parameterList = importFunctionParameterList(dc, clangDecl, params, isVariadic,
-                                              allowNSUIntegerAsInt, argNames,
-                                              genericParams, swiftResultTy);
+  parameterList = importFunctionParameterList(
+      dc, clangDecl, params, isVariadic, allowNSUIntegerAsInt, argNames,
+      genericParams, swiftResultTy, hasSafePointer);
   if (!parameterList)
     return {Type(), false};
 
@@ -2424,13 +2427,6 @@ ClangImporter::Implementation::importParameterType(
         }
       }
     }
-  } else if (auto CAT = dyn_cast<clang::CountAttributedType>(paramTy)) {
-    // Treat as a normal pointer. importBoundsAttributes() will generate a safe
-    // overload later.
-    paramTy = CAT->desugar();
-    if (auto FuncD =
-            dyn_cast<clang::FunctionDecl>(param->getParentFunctionOrMethod()))
-      funcsWithPointerBounds.insert(FuncD);
   } else if (isa<clang::PointerType>(paramTy) &&
              isa<clang::TemplateTypeParmType>(paramTy->getPointeeType())) {
     auto pointeeType = paramTy->getPointeeType();
@@ -2519,6 +2515,7 @@ ClangImporter::Implementation::importParameterType(
     }
   }
 
+  bool isSafePointer = false;
   if (!swiftParamTy) {
     // If this is the throws error parameter, we don't need to convert any
     // NSError** arguments to the sugared NSErrorPointer typealias form,
@@ -2528,11 +2525,11 @@ ClangImporter::Implementation::importParameterType(
     // for the specific case when the throws conversion works, but is not
     // sufficient if it fails. (The correct, overarching fix is ClangImporter
     // being lazier.)
-    auto importedType = importType(paramTy, importKind, addImportDiagnosticFn,
-                                   allowNSUIntegerAsInt, Bridgeability::Full,
-                                   attrs, optionalityOfParam,
-                                   /*resugarNSErrorPointer=*/!paramIsError,
-                                   completionHandlerErrorParamIndex);
+    auto importedType = importType(
+        paramTy, importKind, addImportDiagnosticFn, allowNSUIntegerAsInt,
+        Bridgeability::Full, attrs, optionalityOfParam,
+        /*resugarNSErrorPointer=*/!paramIsError,
+        completionHandlerErrorParamIndex, &isSafePointer);
     if (!importedType)
       return std::nullopt;
 
@@ -2549,8 +2546,8 @@ ClangImporter::Implementation::importParameterType(
   if (isInOut && isDirectUseOfForeignReferenceType(paramTy, swiftParamTy))
     isInOut = false;
 
-  return ImportParameterTypeResult{swiftParamTy, isInOut,
-                                   isParamTypeImplicitlyUnwrapped};
+  return ImportParameterTypeResult{
+      swiftParamTy, isInOut, isParamTypeImplicitlyUnwrapped, isSafePointer};
 }
 
 bool ClangImporter::Implementation::isDefaultArgSafeToImport(
@@ -2661,7 +2658,8 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
     DeclContext *dc, const clang::FunctionDecl *clangDecl,
     ArrayRef<const clang::ParmVarDecl *> params, bool isVariadic,
     bool allowNSUIntegerAsInt, ArrayRef<Identifier> argNames,
-    ArrayRef<GenericTypeParamDecl *> genericParams, Type resultType) {
+    ArrayRef<GenericTypeParamDecl *> genericParams, Type resultType,
+    bool *hasSafePointerParam) {
   // Import the parameters.
   SmallVector<ParamDecl *, 4> parameters;
   unsigned index = 0;
@@ -2701,6 +2699,8 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
     bool isInOut = swiftParamTyOpt->isInOut;
     bool isParamTypeImplicitlyUnwrapped =
         swiftParamTyOpt->isParamTypeImplicitlyUnwrapped;
+    if (swiftParamTyOpt->isSafePointer && hasSafePointerParam)
+      *hasSafePointerParam = true;
 
     // Retrieve the argument name.
     Identifier name;