Skip to content

Support @_predatesConcurrency on import declarations #40679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
2 changes: 1 addition & 1 deletion include/swift/AST/Attr.def
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ SIMPLE_DECL_ATTR(_noAllocation, NoAllocation,

SIMPLE_DECL_ATTR(_predatesConcurrency, PredatesConcurrency,
OnFunc | OnConstructor | OnProtocol | OnGenericType | OnVar | OnSubscript |
OnEnumElement | UserInaccessible |
OnEnumElement | OnImport | UserInaccessible |
ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove,
125)

Expand Down
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,12 @@ NOTE(non_sendable_nominal,none,
NOTE(add_nominal_sendable_conformance,none,
"consider making %0 %1 conform to the 'Sendable' protocol",
(DescriptiveDeclKind, DeclName))
REMARK(add_predates_concurrency_import,none,
"add '@_predatesConcurrency' to %select{suppress|treat}0 "
"'Sendable'-related %select{warnings|errors}0 from module %1"
"%select{| as warnings}0", (bool, Identifier))
REMARK(remove_predates_concurrency_import,none,
"'@_predatesConcurrency' attribute on module %0 is unused", (Identifier))
WARNING(public_decl_needs_sendable,none,
"public %0 %1 does not specify whether it is 'Sendable' or not",
(DescriptiveDeclKind, DeclName))
Expand Down
34 changes: 26 additions & 8 deletions include/swift/AST/Import.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ enum class ImportFlags {
/// implementation detail of this file.
SPIAccessControl = 0x10,

/// The module is imported assuming that the module itself predates
/// concurrency.
PredatesConcurrency = 0x20,

/// Used for DenseMap.
Reserved = 0x80
};
Expand Down Expand Up @@ -533,6 +537,9 @@ struct AttributedImport {
/// Information about the module and access path being imported.
ModuleInfo module;

/// The location of the 'import' keyword, for an explicit import.
SourceLoc importLoc;

/// Flags indicating which attributes of this import are present.
ImportOptions options;

Expand All @@ -543,19 +550,27 @@ struct AttributedImport {
/// Names of explicitly imported SPI groups.
ArrayRef<Identifier> spiGroups;

AttributedImport(ModuleInfo module, ImportOptions options = ImportOptions(),
StringRef filename = {}, ArrayRef<Identifier> spiGroups = {})
: module(module), options(options), sourceFileArg(filename),
spiGroups(spiGroups) {
/// When the import declaration has a `@_predatesConcurrency` annotation, this
/// is the source range covering the annotation.
SourceRange predatesConcurrencyRange;

AttributedImport(ModuleInfo module, SourceLoc importLoc = SourceLoc(),
ImportOptions options = ImportOptions(),
StringRef filename = {}, ArrayRef<Identifier> spiGroups = {},
SourceRange predatesConcurrencyRange = {})
: module(module), importLoc(importLoc), options(options),
sourceFileArg(filename), spiGroups(spiGroups),
predatesConcurrencyRange(predatesConcurrencyRange) {
assert(!(options.contains(ImportFlags::Exported) &&
options.contains(ImportFlags::ImplementationOnly)) ||
options.contains(ImportFlags::Reserved));
}

template<class OtherModuleInfo>
AttributedImport(ModuleInfo module, AttributedImport<OtherModuleInfo> other)
: AttributedImport(module, other.options, other.sourceFileArg,
other.spiGroups) { }
: AttributedImport(module, other.importLoc, other.options,
other.sourceFileArg, other.spiGroups,
other.predatesConcurrencyRange) { }

friend bool operator==(const AttributedImport<ModuleInfo> &lhs,
const AttributedImport<ModuleInfo> &rhs) {
Expand Down Expand Up @@ -705,17 +720,20 @@ struct DenseMapInfo<swift::AttributedImport<ModuleInfo>> {
using ModuleInfoDMI = DenseMapInfo<ModuleInfo>;
using ImportOptionsDMI = DenseMapInfo<swift::ImportOptions>;
using StringRefDMI = DenseMapInfo<StringRef>;
using SourceLocDMI = DenseMapInfo<swift::SourceLoc>;
// We can't include spiGroups in the hash because ArrayRef<Identifier> is not
// DenseMapInfo-able, but we do check that the spiGroups match in isEqual().

static inline AttributedImport getEmptyKey() {
return AttributedImport(ModuleInfoDMI::getEmptyKey(),
SourceLocDMI::getEmptyKey(),
ImportOptionsDMI::getEmptyKey(),
StringRefDMI::getEmptyKey(),
{});
}
static inline AttributedImport getTombstoneKey() {
return AttributedImport(ModuleInfoDMI::getTombstoneKey(),
SourceLocDMI::getEmptyKey(),
ImportOptionsDMI::getTombstoneKey(),
StringRefDMI::getTombstoneKey(),
{});
Expand All @@ -724,8 +742,8 @@ struct DenseMapInfo<swift::AttributedImport<ModuleInfo>> {
return detail::combineHashValue(
ModuleInfoDMI::getHashValue(import.module),
detail::combineHashValue(
ImportOptionsDMI::getHashValue(import.options),
StringRefDMI::getHashValue(import.sourceFileArg)));
ImportOptionsDMI::getHashValue(import.options),
StringRefDMI::getHashValue(import.sourceFileArg)));
}
static bool isEqual(const AttributedImport &a,
const AttributedImport &b) {
Expand Down
12 changes: 12 additions & 0 deletions include/swift/AST/SourceFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ class SourceFile final : public FileUnit {
/// This is \c None until it is filled in by the import resolution phase.
Optional<ArrayRef<AttributedImport<ImportedModule>>> Imports;

/// Which imports have made use of @_predatesConcurrency.
llvm::SmallDenseSet<AttributedImport<ImportedModule>>
PredatesConcurrencyImportsUsed;

/// A unique identifier representing this file; used to mark private decls
/// within the file to keep them from conflicting with other files in the
/// same module.
Expand Down Expand Up @@ -293,6 +297,14 @@ class SourceFile final : public FileUnit {
/// resolution.
void setImports(ArrayRef<AttributedImport<ImportedModule>> imports);

/// Whether the given import has used @_predatesConcurrency.
bool hasImportUsedPredatesConcurrency(
AttributedImport<ImportedModule> import) const;

/// Note that the given import has used @_predatesConcurrency/
void setImportUsedPredatesConcurrency(
AttributedImport<ImportedModule> import);

enum ImportQueryKind {
/// Return the results for testable or private imports.
TestableAndPrivate,
Expand Down
10 changes: 10 additions & 0 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2240,6 +2240,16 @@ SourceFile::setImports(ArrayRef<AttributedImport<ImportedModule>> imports) {
Imports = getASTContext().AllocateCopy(imports);
}

bool SourceFile::hasImportUsedPredatesConcurrency(
AttributedImport<ImportedModule> import) const {
return PredatesConcurrencyImportsUsed.count(import) != 0;
}

void SourceFile::setImportUsedPredatesConcurrency(
AttributedImport<ImportedModule> import) {
PredatesConcurrencyImportsUsed.insert(import);
}

bool HasImplementationOnlyImportsRequest::evaluate(Evaluator &evaluator,
SourceFile *SF) const {
return llvm::any_of(SF->getImports(),
Expand Down
3 changes: 2 additions & 1 deletion lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,8 @@ ImplicitImportInfo CompilerInstance::getImplicitImportInfo() const {
ImportPath::Builder importPath(Context->getIdentifier(moduleStr));
UnloadedImportedModule import(importPath.copyTo(*Context),
/*isScoped=*/false);
imports.AdditionalUnloadedImports.emplace_back(import, options);
imports.AdditionalUnloadedImports.emplace_back(
import, SourceLoc(), options);
};

for (auto &moduleStrAndTestable : frontendOpts.getImplicitImportModuleNames()) {
Expand Down
12 changes: 9 additions & 3 deletions lib/Sema/ImportResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,8 @@ ModuleImplicitImportsRequest::evaluate(Evaluator &evaluator,
!clangImporter->importBridgingHeader(bridgingHeaderPath, module)) {
auto *headerModule = clangImporter->getImportedHeaderModule();
assert(headerModule && "Didn't load bridging header?");
imports.emplace_back(ImportedModule(headerModule), ImportFlags::Exported);
imports.emplace_back(
ImportedModule(headerModule), SourceLoc(), ImportFlags::Exported);
}

// Implicitly import the underlying Clang half of this module if needed.
Expand All @@ -490,7 +491,7 @@ ModuleImplicitImportsRequest::evaluate(Evaluator &evaluator,
ImportPath::Builder importPath(module->getName());
unloadedImports.emplace_back(UnloadedImportedModule(importPath.copyTo(ctx),
/*isScoped=*/false),
ImportFlags::Exported);
SourceLoc(), ImportFlags::Exported);
}

return { ctx.AllocateCopy(imports), ctx.AllocateCopy(unloadedImports) };
Expand Down Expand Up @@ -523,7 +524,7 @@ UnboundImport::UnboundImport(AttributedImport<UnloadedImportedModule> implicit)
/// Create an UnboundImport for a user-written import declaration.
UnboundImport::UnboundImport(ImportDecl *ID)
: import(UnloadedImportedModule(ID->getImportPath(), ID->getImportKind()),
{}),
ID->getStartLoc(), {}),
importLoc(ID->getLoc()), importOrUnderlyingModuleDecl(ID)
{
if (ID->isExported())
Expand All @@ -548,6 +549,11 @@ UnboundImport::UnboundImport(ImportDecl *ID)
spiGroups.append(attrSPIs.begin(), attrSPIs.end());
}
import.spiGroups = ID->getASTContext().AllocateCopy(spiGroups);

if (auto attr = ID->getAttrs().getAttribute<PredatesConcurrencyAttr>()) {
import.options |= ImportFlags::PredatesConcurrency;
import.predatesConcurrencyRange = attr->getRangeWithAt();
}
}

bool UnboundImport::checkNotTautological(const SourceFile &SF) {
Expand Down
99 changes: 88 additions & 11 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "swift/Strings.h"
#include "swift/AST/ASTWalker.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/ImportCache.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/ProtocolConformance.h"
Expand Down Expand Up @@ -674,6 +675,38 @@ static bool hasExplicitSendableConformance(NominalTypeDecl *nominal) {
conformance.getConcrete())->isMissing());
}

/// Find the import that makes the given nominal declaration available.
static Optional<AttributedImport<ImportedModule>> findImportFor(
NominalTypeDecl *nominal, const DeclContext *fromDC) {
// If the nominal type is from the current module, there's no import.
auto nominalModule = nominal->getParentModule();
if (nominalModule == fromDC->getParentModule())
return None;

auto fromSourceFile = fromDC->getParentSourceFile();
if (!fromSourceFile)
return None;

// Look to see if the owning module was directly imported.
for (const auto &import : fromSourceFile->getImports()) {
if (import.module.importedModule == nominalModule)
return import;
}

// Now look for transitive imports.
auto &importCache = nominal->getASTContext().getImportCache();
for (const auto &import : fromSourceFile->getImports()) {
auto &importSet = importCache.getImportSet(import.module.importedModule);
for (const auto &transitive : importSet.getTransitiveImports()) {
if (transitive.importedModule == nominalModule) {
return import;
}
}
}

return None;
}

/// Determine the diagnostic behavior for a Sendable reference to the given
/// nominal type.
DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
Expand All @@ -686,12 +719,12 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(

// Determine whether this nominal type is visible via a @_predatesConcurrency
// import.
ImportDecl *predatesConcurrencyImport = nullptr;
auto import = findImportFor(nominal, fromDC);

// When the type is explicitly non-Sendable...
if (isExplicitlyNonSendable) {
// @_predatesConcurrency imports downgrade the diagnostic to a warning.
if (predatesConcurrencyImport) {
if (import && import->options.contains(ImportFlags::PredatesConcurrency)) {
// FIXME: Note that this @_predatesConcurrency import was "used".
return DiagnosticBehavior::Warning;
}
Expand All @@ -701,10 +734,13 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(

// When the type is implicitly non-Sendable...

// @_predatesConcurrency always suppresses the diagnostic.
if (predatesConcurrencyImport) {
// @_predatesConcurrency suppresses the diagnostic in Swift 5.x, and
// downgrades it to a warning in Swift 6 and later.
if (import && import->options.contains(ImportFlags::PredatesConcurrency)) {
// FIXME: Note that this @_predatesConcurrency import was "used".
return DiagnosticBehavior::Ignore;
return nominalModule->getASTContext().LangOpts.isSwiftVersionAtLeast(6)
? DiagnosticBehavior::Warning
: DiagnosticBehavior::Ignore;
}

return defaultDiagnosticBehavior();
Expand All @@ -729,20 +765,61 @@ static bool diagnoseSingleNonSendableType(

bool wasSuppressed = diagnose(type, behavior);

// If this type was imported from another module, try to find the
// corresponding import.
Optional<AttributedImport<swift::ImportedModule>> import;
SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
if (nominal && nominal->getParentModule() != module) {
import = findImportFor(nominal, fromContext.fromDC);
}

if (behavior == DiagnosticBehavior::Ignore || wasSuppressed) {
// Don't emit any other diagnostics.
} else if (type->is<FunctionType>()) {
ctx.Diags.diagnose(loc, diag::nonsendable_function_type);
} else if (nominal && nominal->getParentModule() == module &&
(isa<StructDecl>(nominal) || isa<EnumDecl>(nominal))) {
auto note = nominal->diagnose(
diag::add_nominal_sendable_conformance,
nominal->getDescriptiveKind(), nominal->getName());
addSendableFixIt(nominal, note, /*unchecked=*/false);
} else if (nominal && nominal->getParentModule() == module) {
// If the nominal type is in the current module, suggest adding
// `Sendable` if it might make sense. Otherwise, just complain.
if (isa<StructDecl>(nominal) || isa<EnumDecl>(nominal)) {
auto note = nominal->diagnose(
diag::add_nominal_sendable_conformance,
nominal->getDescriptiveKind(), nominal->getName());
addSendableFixIt(nominal, note, /*unchecked=*/false);
} else {
nominal->diagnose(
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
nominal->getName());
}
} else if (nominal) {
// Note which nominal type does not conform to `Sendable`.
nominal->diagnose(
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
nominal->getName());

// If we found the import that makes this nominal type visible, remark
// that it can be @_predatesConcurrency import.
// Only emit this remark once per source file, because it can happen a
// lot.
if (import && !import->options.contains(ImportFlags::PredatesConcurrency) &&
import->importLoc.isValid() && sourceFile &&
!sourceFile->hasImportUsedPredatesConcurrency(*import)) {
SourceLoc importLoc = import->importLoc;
ctx.Diags.diagnose(
importLoc, diag::add_predates_concurrency_import,
ctx.LangOpts.isSwiftVersionAtLeast(6),
nominal->getParentModule()->getName())
.fixItInsert(importLoc, "@_predatesConcurrency ");

sourceFile->setImportUsedPredatesConcurrency(*import);
}
}

// If we found an import that makes this nominal type visible, and that
// was a @_predatesConcurrency import, note that we have made use of the
// attribute.
if (import && import->options.contains(ImportFlags::PredatesConcurrency) &&
sourceFile) {
sourceFile->setImportUsedPredatesConcurrency(*import);
}

return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
Expand Down
19 changes: 19 additions & 0 deletions lib/Sema/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,23 @@ void swift::performTypeChecking(SourceFile &SF) {
TypeCheckSourceFileRequest{&SF}, {});
}

/// If any of the imports in this source file was @_predatesConcurrency but
/// there were no diagnostics downgraded or suppressed due to that
/// @_predatesConcurrency, suggest that the attribute be removed.
static void diagnoseUnnecessaryPredatesConcurrencyImports(SourceFile &sf) {
ASTContext &ctx = sf.getASTContext();
for (const auto &import : sf.getImports()) {
if (import.options.contains(ImportFlags::PredatesConcurrency) &&
import.importLoc.isValid() &&
!sf.hasImportUsedPredatesConcurrency(import)) {
ctx.Diags.diagnose(
import.importLoc, diag::remove_predates_concurrency_import,
import.module.importedModule->getName())
.fixItRemove(import.predatesConcurrencyRange);
}
}
}

evaluator::SideEffect
TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
assert(SF && "Source file cannot be null!");
Expand Down Expand Up @@ -305,6 +322,8 @@ TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
typeCheckDelayedFunctions(*SF);
}

diagnoseUnnecessaryPredatesConcurrencyImports(*SF);

// Check to see if there's any inconsistent @_implementationOnly imports.
evaluateOrDefault(
Ctx.evaluator,
Expand Down
1 change: 1 addition & 0 deletions test/ClangImporter/objc_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// REQUIRES: concurrency
import Foundation
import ObjCConcurrency
// expected-remark@-1{{add '@_predatesConcurrency' to suppress 'Sendable'-related warnings from module 'ObjCConcurrency'}}

if #available(SwiftStdlib 5.5, *) {

Expand Down
Loading