Skip to content

[region-isolation] Suppress non-Sendable region isolation errors appropriately when the type is imported by using @preconcurrency import. #73212

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 1 commit into from
Apr 24, 2024
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
6 changes: 6 additions & 0 deletions include/swift/Sema/Concurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@
namespace swift {

class SourceFile;
class NominalTypeDecl;

/// If any of the imports in this source file was @preconcurrency but there were
/// no diagnostics downgraded or suppressed due to that @preconcurrency, suggest
/// that the attribute be removed.
void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);

/// Determine whether the given nominal type has an explicit Sendable
/// conformance (regardless of its availability).
bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
bool applyModuleDefault = true);

} // namespace swift

#endif
81 changes: 66 additions & 15 deletions lib/SILOptimizer/Mandatory/TransferNonSendable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "swift/AST/ASTWalker.h"
#include "swift/AST/DiagnosticsSIL.h"
#include "swift/AST/Expr.h"
#include "swift/AST/ProtocolConformance.h"
#include "swift/AST/SourceFile.h"
#include "swift/AST/Type.h"
#include "swift/Basic/FrozenMultiMap.h"
#include "swift/Basic/ImmutablePointerSet.h"
Expand All @@ -35,6 +37,7 @@
#include "swift/SILOptimizer/PassManager/Transforms.h"
#include "swift/SILOptimizer/Utils/PartitionUtils.h"
#include "swift/SILOptimizer/Utils/VariableNameUtils.h"
#include "swift/Sema/Concurrency.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/Support/Debug.h"

Expand All @@ -55,6 +58,33 @@ using Region = PartitionPrimitives::Region;
// MARK: Utilities
//===----------------------------------------------------------------------===//

static std::optional<DiagnosticBehavior>
getDiagnosticBehaviorLimitForValue(SILValue value) {
auto *nom = value->getType().getNominalOrBoundGenericNominal();
if (!nom)
return {};

auto declRef = value->getFunction()->getDeclRef();
if (!declRef)
return {};

auto *fromDC = declRef.getInnermostDeclContext();
auto attributedImport = nom->findImport(fromDC);
if (!attributedImport ||
!attributedImport->options.contains(ImportFlags::Preconcurrency))
return {};

if (auto *sourceFile = fromDC->getParentSourceFile())
sourceFile->setImportUsedPreconcurrency(*attributedImport);

if (hasExplicitSendableConformance(nom))
return DiagnosticBehavior::Warning;

return attributedImport->module.importedModule->isConcurrencyChecked()
? DiagnosticBehavior::Warning
: DiagnosticBehavior::Ignore;
}

static Expr *inferArgumentExprFromApplyExpr(ApplyExpr *sourceApply,
FullApplySite fai,
const Operand *op) {
Expand Down Expand Up @@ -438,14 +468,19 @@ class UseAfterTransferDiagnosticEmitter {
emitUnknownPatternError();
}

std::optional<DiagnosticBehavior> getBehaviorLimit() const {
return getDiagnosticBehaviorLimitForValue(transferOp->get());
}

void
emitNamedIsolationCrossingError(SILLocation loc, Identifier name,
SILIsolationInfo namedValuesIsolationInfo,
ApplyIsolationCrossing isolationCrossing) {
// Emit the short error.
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
name)
.highlight(loc.getSourceRange());
.highlight(loc.getSourceRange())
.limitBehaviorIf(getBehaviorLimit());

// Then emit the note with greater context.
SmallString<64> descriptiveKindStr;
Expand All @@ -466,7 +501,8 @@ class UseAfterTransferDiagnosticEmitter {
loc, diag::regionbasedisolation_transfer_yields_race_with_isolation,
inferredType, isolationCrossing.getCallerIsolation(),
isolationCrossing.getCalleeIsolation())
.highlight(loc.getSourceRange());
.highlight(loc.getSourceRange())
.limitBehaviorIf(getBehaviorLimit());
emitRequireInstDiagnostics();
}

Expand All @@ -475,7 +511,8 @@ class UseAfterTransferDiagnosticEmitter {
// Emit the short error.
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
name)
.highlight(loc.getSourceRange());
.highlight(loc.getSourceRange())
.limitBehaviorIf(getBehaviorLimit());

// Then emit the note with greater context.
diagnoseNote(
Expand All @@ -493,7 +530,8 @@ class UseAfterTransferDiagnosticEmitter {
diag::
regionbasedisolation_transfer_yields_race_stronglytransferred_binding,
inferredType)
.highlight(loc.getSourceRange());
.highlight(loc.getSourceRange())
.limitBehaviorIf(getBehaviorLimit());
emitRequireInstDiagnostics();
}

Expand All @@ -502,7 +540,8 @@ class UseAfterTransferDiagnosticEmitter {
diagnoseError(loc,
diag::regionbasedisolation_transfer_yields_race_no_isolation,
inferredType)
.highlight(loc.getSourceRange());
.highlight(loc.getSourceRange())
.limitBehaviorIf(getBehaviorLimit());
emitRequireInstDiagnostics();
}

Expand All @@ -513,7 +552,8 @@ class UseAfterTransferDiagnosticEmitter {
// Emit the short error.
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
name)
.highlight(loc.getSourceRange());
.highlight(loc.getSourceRange())
.limitBehaviorIf(getBehaviorLimit());

SmallString<64> descriptiveKindStr;
{
Expand All @@ -535,13 +575,15 @@ class UseAfterTransferDiagnosticEmitter {
diagnoseError(loc, diag::regionbasedisolation_isolated_capture_yields_race,
inferredType, isolationCrossing.getCalleeIsolation(),
isolationCrossing.getCallerIsolation())
.highlight(loc.getSourceRange());
.highlight(loc.getSourceRange())
.limitBehaviorIf(getBehaviorLimit());
emitRequireInstDiagnostics();
}

void emitUnknownPatternError() {
diagnoseError(transferOp->getUser(),
diag::regionbasedisolation_unknown_pattern);
diag::regionbasedisolation_unknown_pattern)
.limitBehaviorIf(getBehaviorLimit());
}

private:
Expand Down Expand Up @@ -941,20 +983,26 @@ class TransferNonTransferrableDiagnosticEmitter {
return info.nonTransferrable.dyn_cast<SILInstruction *>();
}

std::optional<DiagnosticBehavior> getBehaviorLimit() const {
return getDiagnosticBehaviorLimitForValue(info.transferredOperand->get());
}

/// Return the isolation region info for \p getNonTransferrableValue().
SILIsolationInfo getIsolationRegionInfo() const {
return info.isolationRegionInfo;
}

void emitUnknownPatternError() {
diagnoseError(getOperand()->getUser(),
diag::regionbasedisolation_unknown_pattern);
diag::regionbasedisolation_unknown_pattern)
.limitBehaviorIf(getBehaviorLimit());
}

void emitUnknownUse(SILLocation loc) {
// TODO: This will eventually be an unknown pattern error.
diagnoseError(
loc, diag::regionbasedisolation_task_or_actor_isolated_transferred);
diagnoseError(loc,
diag::regionbasedisolation_task_or_actor_isolated_transferred)
.limitBehaviorIf(getBehaviorLimit());
}

void emitFunctionArgumentApply(SILLocation loc, Type type,
Expand All @@ -967,7 +1015,8 @@ class TransferNonTransferrableDiagnosticEmitter {
diagnoseError(loc, diag::regionbasedisolation_arg_transferred,
StringRef(descriptiveKindStr), type,
crossing.getCalleeIsolation())
.highlight(getOperand()->getUser()->getLoc().getSourceRange());
.highlight(getOperand()->getUser()->getLoc().getSourceRange())
.limitBehaviorIf(getBehaviorLimit());
}

void emitNamedFunctionArgumentClosure(SILLocation loc, Identifier name,
Expand Down Expand Up @@ -995,13 +1044,15 @@ class TransferNonTransferrableDiagnosticEmitter {
auto diag =
diag::regionbasedisolation_arg_passed_to_strongly_transferred_param;
diagnoseError(loc, diag, descriptiveKindStr, type)
.highlight(getOperand()->getUser()->getLoc().getSourceRange());
.highlight(getOperand()->getUser()->getLoc().getSourceRange())
.limitBehaviorIf(getBehaviorLimit());
}

void emitNamedOnlyError(SILLocation loc, Identifier name) {
diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
name)
.highlight(getOperand()->getUser()->getLoc().getSourceRange());
.highlight(getOperand()->getUser()->getLoc().getSourceRange())
.limitBehaviorIf(getBehaviorLimit());
}

void emitNamedIsolation(SILLocation loc, Identifier name,
Expand Down Expand Up @@ -1176,7 +1227,7 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
if (!isolation) {
// Otherwise, emit a "we don't know error" that tells the user to file a
// bug.
diagnoseError(op->getUser(), diag::regionbasedisolation_unknown_pattern);
diagnosticEmitter.emitUnknownPatternError();
return false;
}
assert(isolation && "Expected non-null");
Expand Down
4 changes: 2 additions & 2 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -789,8 +789,8 @@ SendableCheckContext::implicitSendableDiagnosticBehavior() const {

/// Determine whether the given nominal type has an explicit Sendable
/// conformance (regardless of its availability).
static bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
bool applyModuleDefault = true) {
bool swift::hasExplicitSendableConformance(NominalTypeDecl *nominal,
bool applyModuleDefault) {
ASTContext &ctx = nominal->getASTContext();
auto nominalModule = nominal->getParentModule();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

public class NonSendableKlass {
public init() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

public class NonSendableKlass {
public init() {}
}

public class ExplicitlyNonSendableKlass {
public init() {}
}

@available(*, unavailable)
extension ExplicitlyNonSendableKlass: Sendable {}
Loading