Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsFrontend.def
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@ ERROR(no_swift_sources_with_embedded,none,
ERROR(package_cmo_requires_library_evolution, none,
"Library evolution must be enabled for Package CMO", ())

WARNING(internal_bridging_header_without_library_evolution,none,
"using internal bridging headers without library evolution can cause instability", ())

ERROR(experimental_not_supported_in_production,none,
"experimental feature '%0' cannot be enabled in production compiler",
(StringRef))
Expand Down Expand Up @@ -636,6 +639,8 @@ NOTE(dependency_scan_unexpected_variant_module_map_note, none,
NOTE(dependency_scan_unexpected_variant_extra_arg_note, none,
"%select{first|second}0 module command-line has extra argument: '%1'", (bool, StringRef))

ERROR(bridging_header_and_pch_internal_mismatch,none,
"bridging header and precompiled header options mismatch on internal vs. public import", ())

#define UNDEFINE_DIAGNOSTIC_MACROS
#include "DefineDiagnosticMacros.h"
13 changes: 9 additions & 4 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2702,13 +2702,13 @@ NOTE(module_imported_here,none,
NOTE(decl_import_via_here,none,
"%kind0 imported as "
"'%select{private|fileprivate|internal|package|%ERROR|%ERROR}1' "
"from %2 here",
(const Decl *, AccessLevel, const ModuleDecl*))
"from %select{%2 here|bridging header}3",
(const Decl *, AccessLevel, const ModuleDecl*, bool))
NOTE(decl_import_via_local,none,
"%kind0 is imported by this file as "
"'%select{private|fileprivate|internal|package|%ERROR|%ERROR}1' "
"from %2",
(const Decl *, AccessLevel, const ModuleDecl*))
"from %select{%2|bridging header}3",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there may be more resilience diagnostics that will need to be updated, but it seems like this is a good start

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps! They'll come out as the weird __ObjC module if we don't customize them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new kind of DisallowedOriginKind may do the trick here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that could help here, yes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this in b7a8349

(const Decl *, AccessLevel, const ModuleDecl*, bool))

// Opaque return types
ERROR(opaque_type_invalid_constraint,none,
Expand Down Expand Up @@ -3834,6 +3834,7 @@ ERROR(decl_from_hidden_module,none,
"%2 was imported for SPI only|"
"%2 was not imported by this file|"
"C++ types from imported module %2 do not support library evolution|"
"it was imported via the internal bridging header|"
"%2 was not imported publicly}3",
(const Decl *, unsigned, Identifier, unsigned))
ERROR(typealias_desugars_to_type_from_hidden_module,none,
Expand All @@ -3850,6 +3851,7 @@ ERROR(typealias_desugars_to_type_from_hidden_module,none,
"%4 was imported for SPI only|"
"%4 was not imported by this file|"
"C++ types from imported module %4 do not support library evolution|"
"it was imported via the internal bridging header|"
"%4 was not imported publicly}5",
(const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))
ERROR(conformance_from_implementation_only_module,none,
Expand All @@ -3864,6 +3866,7 @@ ERROR(conformance_from_implementation_only_module,none,
"%3 was imported for SPI only|"
"%3 was not imported by this file|"
"C++ types from imported module %3 do not support library evolution|"
"it was imported via the internal bridging header|"
"%3 was not imported publicly}4",
(Type, Identifier, unsigned, Identifier, unsigned))
NOTE(assoc_conformance_from_implementation_only_module,none,
Expand Down Expand Up @@ -7317,6 +7320,7 @@ ERROR(inlinable_decl_ref_from_hidden_module,
"%2 was imported for SPI only|"
"%2 was not imported by this file|"
"C++ APIs from imported module %2 do not support library evolution|"
"it was imported via the internal bridging header|"
"%2 was not imported publicly}3",
(const ValueDecl *, unsigned, Identifier, unsigned))

Expand All @@ -7328,6 +7332,7 @@ ERROR(inlinable_typealias_desugars_to_type_from_hidden_module,
"%4 was imported for SPI only|"
"%4 was not imported by this file|"
"C++ types from imported module %4 do not support library evolution|"
"it was imported via the internal bridging header|"
"%4 was not imported publicly}5",
(const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))

Expand Down
4 changes: 4 additions & 0 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,10 @@ namespace swift {
/// The bridging header PCH file.
std::string BridgingHeaderPCH;

/// Whether the bridging header and PCH file are considered to be
/// internal imports.
bool BridgingHeaderIsInternal = false;

/// When automatically generating a precompiled header from the bridging
/// header, place it in this directory.
std::string PrecompiledHeaderOutputDir;
Expand Down
8 changes: 6 additions & 2 deletions include/swift/Frontend/FrontendOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,16 @@ class FrontendOptions {

bool isOutputFileDirectory() const;

/// An Objective-C header to import and make implicitly visible.
/// A C header to import and make implicitly visible.
std::string ImplicitObjCHeaderPath;

/// An Objective-C pch to import and make implicitly visible.
/// A C pch to import and make implicitly visible.
std::string ImplicitObjCPCHPath;

/// Whether the imported C header or precompiled header is considered
/// an internal import (vs. the default, a public import).
bool ImportHeaderAsInternal = false;

/// The map of aliases and real names of imported or referenced modules.
llvm::StringMap<std::string> ModuleAliasMap;

Expand Down
17 changes: 13 additions & 4 deletions include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -347,16 +347,25 @@ def import_underlying_module : Flag<["-"], "import-underlying-module">,
Flags<[FrontendOption, NoInteractiveOption]>,
HelpText<"Implicitly imports the Objective-C half of a module">;

def import_objc_header : Separate<["-"], "import-objc-header">,
Flags<[FrontendOption, HelpHidden, ArgumentIsPath]>,
HelpText<"Implicitly imports an Objective-C header file">;
def import_bridging_header : Separate<["-"], "import-bridging-header">,
Flags<[FrontendOption, ArgumentIsPath]>,
HelpText<"Implicitly imports a C header file">;
def import_objc_header : Separate<["-"], "import-objc-header">,
Flags<[FrontendOption, HelpHidden, ArgumentIsPath]>,
Alias<import_objc_header>;
Alias<import_bridging_header>;

def internal_import_bridging_header : Separate<["-"], "internal-import-bridging-header">,
Flags<[FrontendOption, ArgumentIsPath]>,
HelpText<"Implicitly imports a C header file as an internal import">;

def import_pch : Separate<["-"], "import-pch">,
Flags<[FrontendOption, HelpHidden, ArgumentIsPath]>,
HelpText<"Import bridging header PCH file">;

def internal_import_pch : Separate<["-"], "internal-import-pch">,
Flags<[FrontendOption, HelpHidden, ArgumentIsPath]>,
HelpText<"Import bridging header PCH file as internal">;

def pch_output_dir: Separate<["-"], "pch-output-dir">,
Flags<[FrontendOption, HelpHidden, ArgumentIsPath]>,
HelpText<"Directory to persist automatically created precompiled bridging headers">;
Expand Down
4 changes: 3 additions & 1 deletion lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2991,7 +2991,9 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
// they are recommended over indirect imports.
if ((!restrictiveImport.has_value() ||
restrictiveImport->accessLevel < AccessLevel::Public) &&
imports.isImportedBy(targetModule, getParentModule()))
!(restrictiveImport &&
restrictiveImport->module.importedModule->isClangHeaderImportModule()) &&
imports.isImportedBy(targetModule, getParentModule()))
return std::nullopt;

return restrictiveImport;
Expand Down
8 changes: 7 additions & 1 deletion lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2714,6 +2714,7 @@ ClangImporter::Implementation::Implementation(
DisableSwiftBridgeAttr(ctx.ClangImporterOpts.DisableSwiftBridgeAttr),
BridgingHeaderExplicitlyRequested(
!ctx.ClangImporterOpts.BridgingHeader.empty()),
BridgingHeaderIsInternal(ctx.ClangImporterOpts.BridgingHeaderIsInternal),
DisableOverlayModules(ctx.ClangImporterOpts.DisableOverlayModules),
EnableClangSPI(ctx.ClangImporterOpts.EnableClangSPI),
IsReadingBridgingPCH(false),
Expand Down Expand Up @@ -3762,8 +3763,13 @@ ImportDecl *swift::createImportDecl(ASTContext &Ctx,
auto *ID = ImportDecl::create(Ctx, DC, SourceLoc(),
ImportKind::Module, SourceLoc(),
importPath.get(), ClangN);
if (IsExported)
if (Ctx.ClangImporterOpts.BridgingHeaderIsInternal) {
ID->getAttrs().add(
new (Ctx) AccessControlAttr(SourceLoc(), SourceRange(),
AccessLevel::Internal, /*implicit=*/true));
} else if (IsExported) {
ID->getAttrs().add(new (Ctx) ExportedAttr(/*IsImplicit=*/false));
}
return ID;
}

Expand Down
1 change: 1 addition & 0 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
const bool ImportForwardDeclarations;
const bool DisableSwiftBridgeAttr;
const bool BridgingHeaderExplicitlyRequested;
const bool BridgingHeaderIsInternal;
const bool DisableOverlayModules;
const bool EnableClangSPI;

Expand Down
7 changes: 5 additions & 2 deletions lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ static void validateLegacyUnsupportedArgs(DiagnosticEngine &diags,

static void validateBridgingHeaderArgs(DiagnosticEngine &diags,
const ArgList &args) {
if (!args.hasArgNoClaim(options::OPT_import_objc_header))
if (!args.hasArgNoClaim(options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header))
return;

if (args.hasArgNoClaim(options::OPT_import_underlying_module))
Expand Down Expand Up @@ -1521,7 +1522,9 @@ void Driver::buildActions(SmallVectorImpl<const Action *> &TopLevelActions,
if (Args.hasFlag(options::OPT_enable_bridging_pch,
options::OPT_disable_bridging_pch,
true)) {
if (Arg *A = Args.getLastArg(options::OPT_import_objc_header)) {
if (Arg *A = Args.getLastArg(
options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header)) {
StringRef Value = A->getValue();
auto Ty = TC.lookupTypeForExtension(llvm::sys::path::extension(Value));
if (Ty == file_types::TY_ClangHeader) {
Expand Down
32 changes: 23 additions & 9 deletions lib/Driver/ToolChains.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -522,19 +522,28 @@ ToolChain::constructInvocation(const CompileJobAction &job,
addCommonFrontendArgs(context.OI, context.Output, context.Args, Arguments);
addRuntimeLibraryFlags(context.OI, Arguments);

// Pass along an -import-objc-header arg, replacing the argument with the name
// of any input PCH to the current action if one is present.
if (context.Args.hasArgNoClaim(options::OPT_import_objc_header)) {
// Pass along an -(internal-)?import-bridging-header arg, replacing the
// argument with the name of any input PCH to the current action if one is
// present.
if (context.Args.hasArgNoClaim(options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header)) {
bool ForwardAsIs = true;
bool bridgingPCHIsEnabled =
context.Args.hasFlag(options::OPT_enable_bridging_pch,
options::OPT_disable_bridging_pch, true);
bool usePersistentPCH = bridgingPCHIsEnabled &&
context.Args.hasArg(options::OPT_pch_output_dir);
bool isInternalImport = context.Args.getLastArgNoClaim(
options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header)
->getOption().getID() == options::OPT_internal_import_bridging_header;
if (!usePersistentPCH) {
for (auto *IJ : context.Inputs) {
if (!IJ->getOutput().getAnyOutputForType(file_types::TY_PCH).empty()) {
Arguments.push_back("-import-objc-header");
if (isInternalImport)
Arguments.push_back("-internal-import-bridging-header");
else
Arguments.push_back("-import-bridging-header");
addInputsOfType(Arguments, context.Inputs, context.Args,
file_types::TY_PCH);
ForwardAsIs = false;
Expand All @@ -543,7 +552,8 @@ ToolChain::constructInvocation(const CompileJobAction &job,
}
}
if (ForwardAsIs) {
context.Args.AddLastArg(Arguments, options::OPT_import_objc_header);
context.Args.AddLastArg(Arguments, options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header);
}
if (usePersistentPCH) {
context.Args.AddLastArg(Arguments, options::OPT_pch_output_dir);
Expand Down Expand Up @@ -972,7 +982,8 @@ ToolChain::constructInvocation(const InterpretJobAction &job,
addCommonFrontendArgs(context.OI, context.Output, context.Args, Arguments);
addRuntimeLibraryFlags(context.OI, Arguments);

context.Args.AddLastArg(Arguments, options::OPT_import_objc_header);
context.Args.AddLastArg(Arguments, options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header);

context.Args.AddLastArg(Arguments, options::OPT_parse_sil);

Expand Down Expand Up @@ -1233,7 +1244,8 @@ ToolChain::constructInvocation(const MergeModuleJobAction &job,
options::OPT_omit_extension_block_symbols);
context.Args.AddLastArg(Arguments, options::OPT_symbol_graph_minimum_access_level);

context.Args.AddLastArg(Arguments, options::OPT_import_objc_header);
context.Args.AddLastArg(Arguments, options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header);

Arguments.push_back("-module-name");
Arguments.push_back(context.Args.MakeArgString(context.OI.ModuleName));
Expand Down Expand Up @@ -1276,7 +1288,8 @@ ToolChain::constructInvocation(const VerifyModuleInterfaceJobAction &job,
file_types::TY_SerializedDiagnostics,
"-serialize-diagnostics-path");

context.Args.AddLastArg(Arguments, options::OPT_import_objc_header);
context.Args.AddLastArg(Arguments, options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header);

Arguments.push_back("-module-name");
Arguments.push_back(context.Args.MakeArgString(context.OI.ModuleName));
Expand Down Expand Up @@ -1342,7 +1355,8 @@ ToolChain::constructInvocation(const REPLJobAction &job,
addCommonFrontendArgs(context.OI, context.Output, context.Args, FrontendArgs);
addRuntimeLibraryFlags(context.OI, FrontendArgs);

context.Args.AddLastArg(FrontendArgs, options::OPT_import_objc_header);
context.Args.AddLastArg(FrontendArgs, options::OPT_import_bridging_header,
options::OPT_internal_import_bridging_header);
context.Args.addAllArgs(FrontendArgs,
{options::OPT_framework, options::OPT_L});
ToolChain::addLinkedLibArgs(context.Args, FrontendArgs);
Expand Down
30 changes: 26 additions & 4 deletions lib/Frontend/ArgsToFrontendOptionsConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -913,18 +913,40 @@ static inline bool isPCHFilenameExtension(StringRef path) {

void ArgsToFrontendOptionsConverter::computeImportObjCHeaderOptions() {
using namespace options;
if (const Arg *A = Args.getLastArgNoClaim(OPT_import_objc_header)) {
// Legacy support for passing PCH file through `-import-objc-header`.
bool hadNormalBridgingHeader = false;
if (const Arg *A = Args.getLastArgNoClaim(
OPT_import_bridging_header,
OPT_internal_import_bridging_header)) {
// Legacy support for passing PCH file through `-import-bridging-header`.
if (isPCHFilenameExtension(A->getValue()))
Opts.ImplicitObjCPCHPath = A->getValue();
else
Opts.ImplicitObjCHeaderPath = A->getValue();
// If `-import-object-header` is used, it means the module has a direct
// If `-import-bridging-header` is used, it means the module has a direct
// bridging header dependency and it can be serialized into binary module.
Opts.ModuleHasBridgingHeader |= true;

Opts.ImportHeaderAsInternal =
A->getOption().getID() == OPT_internal_import_bridging_header;

hadNormalBridgingHeader = true;
}
if (const Arg *A = Args.getLastArgNoClaim(OPT_import_pch))
if (const Arg *A = Args.getLastArgNoClaim(OPT_import_pch,
OPT_internal_import_pch)) {
Opts.ImplicitObjCPCHPath = A->getValue();

bool importAsInternal = A->getOption().getID() == OPT_internal_import_pch;

/// Don't let the bridging-header and precompiled-header options differ in
/// whether they are treated as internal or public imports.
if (hadNormalBridgingHeader &&
importAsInternal != Opts.ImportHeaderAsInternal) {
Diags.diagnose(SourceLoc(),
diag::bridging_header_and_pch_internal_mismatch);
}

Opts.ImportHeaderAsInternal = importAsInternal;
}
}
void ArgsToFrontendOptionsConverter::
computeImplicitImportModuleNames(OptSpecifier id, bool isTestable) {
Expand Down
27 changes: 25 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ setBridgingHeaderFromFrontendOptions(ClangImporterOptions &ImporterOpts,
if (!FrontendOpts.InputsAndOutputs.hasInputs())
return;

ImporterOpts.BridgingHeaderIsInternal = FrontendOpts.ImportHeaderAsInternal;

// If we aren't asked to output a bridging header, we don't need to set this.
if (ImporterOpts.PrecompiledHeaderOutputDir.empty())
return;
Expand Down Expand Up @@ -2109,10 +2111,31 @@ static bool ParseClangImporterArgs(ClangImporterOptions &Opts, ArgList &Args,
else if (Args.hasArg(OPT_emit_pcm) || Args.hasArg(OPT_dump_pcm))
Opts.Mode = ClangImporterOptions::Modes::PrecompiledModule;

if (auto *A = Args.getLastArg(OPT_import_objc_header))
bool hadNormalBridgingHeader = false;
if (auto *A = Args.getLastArg(OPT_import_bridging_header,
OPT_internal_import_bridging_header)) {
Opts.BridgingHeader = A->getValue();
if (auto *A = Args.getLastArg(OPT_import_pch))
Opts.BridgingHeaderIsInternal =
A->getOption().getID() == OPT_internal_import_bridging_header;
}
if (auto *A = Args.getLastArg(OPT_import_pch, OPT_internal_import_pch)) {
Opts.BridgingHeaderPCH = A->getValue();
bool importAsInternal = A->getOption().getID() == OPT_internal_import_pch;
if (hadNormalBridgingHeader &&
importAsInternal != Opts.BridgingHeaderIsInternal) {
Diags.diagnose(SourceLoc(),
diag::bridging_header_and_pch_internal_mismatch);
}
Opts.BridgingHeaderIsInternal = importAsInternal;
}

// Until we have some checking in place, internal bridging headers are a
// bit unsafe without library evolution.
if (Opts.BridgingHeaderIsInternal && !FrontendOpts.EnableLibraryEvolution) {
Diags.diagnose(SourceLoc(),
diag::internal_bridging_header_without_library_evolution);
}

Opts.DisableSwiftBridgeAttr |= Args.hasArg(OPT_disable_swift_bridge_attr);

Opts.DisableOverlayModules |= Args.hasArg(OPT_emit_imported_modules);
Expand Down
3 changes: 2 additions & 1 deletion lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ SerializationOptions CompilerInvocation::computeSerializationOptions(
serializationOpts.DocOutputPath = outs.ModuleDocOutputPath;
serializationOpts.SourceInfoOutputPath = outs.ModuleSourceInfoOutputPath;
serializationOpts.GroupInfoPath = opts.GroupInfoPath.c_str();
if (opts.ModuleHasBridgingHeader && !outs.ModuleOutputPath.empty())
if (opts.ModuleHasBridgingHeader && !outs.ModuleOutputPath.empty() &&
!opts.ImportHeaderAsInternal)
serializationOpts.SerializeBridgingHeader = true;
// For batch mode, emit empty header path as placeholder.
if (serializationOpts.SerializeBridgingHeader &&
Expand Down
3 changes: 3 additions & 0 deletions lib/Option/features.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
},
{
"name": "Isystem"
},
{
"name": "internal-import-bridging-header"
}
]
}
Loading