Skip to content

Commit a65d2ea

Browse files
committed
Sema: Avoid emitting fixit to prefix import with @preconcurrency in swiftinterfaces.
Resolves rdar://105711934
1 parent 77feef5 commit a65d2ea

File tree

4 files changed

+83
-68
lines changed

4 files changed

+83
-68
lines changed

lib/Sema/TypeCheckConcurrency.cpp

+69-35
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,19 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
793793
return defaultBehavior;
794794
}
795795

796+
static bool shouldDiagnosePreconcurrencyImports(SourceFile &sf) {
797+
switch (sf.Kind) {
798+
case SourceFileKind::Interface:
799+
case SourceFileKind::SIL:
800+
return false;
801+
802+
case SourceFileKind::Library:
803+
case SourceFileKind::Main:
804+
case SourceFileKind::MacroExpansion:
805+
return true;
806+
}
807+
}
808+
796809
bool swift::diagnoseSendabilityErrorBasedOn(
797810
NominalTypeDecl *nominal, SendableCheckContext fromContext,
798811
llvm::function_ref<bool(DiagnosticBehavior)> diagnose) {
@@ -806,49 +819,70 @@ bool swift::diagnoseSendabilityErrorBasedOn(
806819

807820
bool wasSuppressed = diagnose(behavior);
808821

809-
bool emittedDiagnostics =
810-
behavior != DiagnosticBehavior::Ignore && !wasSuppressed;
811-
812-
// When the type is explicitly Sendable *or* explicitly non-Sendable, we
813-
// assume it has been audited and `@preconcurrency` is not recommended even
814-
// though it would actually affect the diagnostic.
815-
bool nominalIsImportedAndHasImplicitSendability =
816-
nominal &&
817-
nominal->getParentModule() != fromContext.fromDC->getParentModule() &&
818-
!hasExplicitSendableConformance(nominal);
819-
820-
if (emittedDiagnostics && nominalIsImportedAndHasImplicitSendability) {
821-
// This type was imported from another module; try to find the
822-
// corresponding import.
823-
Optional<AttributedImport<swift::ImportedModule>> import;
824-
SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
825-
if (sourceFile) {
826-
import = findImportFor(nominal, fromContext.fromDC);
827-
}
828-
829-
// If we found the import that makes this nominal type visible, remark
830-
// that it can be @preconcurrency import.
831-
// Only emit this remark once per source file, because it can happen a
832-
// lot.
833-
if (import && !import->options.contains(ImportFlags::Preconcurrency) &&
834-
import->importLoc.isValid() && sourceFile &&
835-
!sourceFile->hasImportUsedPreconcurrency(*import)) {
836-
SourceLoc importLoc = import->importLoc;
837-
ASTContext &ctx = nominal->getASTContext();
822+
SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
823+
if (sourceFile && shouldDiagnosePreconcurrencyImports(*sourceFile)) {
824+
bool emittedDiagnostics =
825+
behavior != DiagnosticBehavior::Ignore && !wasSuppressed;
826+
827+
// When the type is explicitly Sendable *or* explicitly non-Sendable, we
828+
// assume it has been audited and `@preconcurrency` is not recommended even
829+
// though it would actually affect the diagnostic.
830+
bool nominalIsImportedAndHasImplicitSendability =
831+
nominal &&
832+
nominal->getParentModule() != fromContext.fromDC->getParentModule() &&
833+
!hasExplicitSendableConformance(nominal);
834+
835+
if (emittedDiagnostics && nominalIsImportedAndHasImplicitSendability) {
836+
// This type was imported from another module; try to find the
837+
// corresponding import.
838+
Optional<AttributedImport<swift::ImportedModule>> import =
839+
findImportFor(nominal, fromContext.fromDC);
840+
841+
// If we found the import that makes this nominal type visible, remark
842+
// that it can be @preconcurrency import.
843+
// Only emit this remark once per source file, because it can happen a
844+
// lot.
845+
if (import && !import->options.contains(ImportFlags::Preconcurrency) &&
846+
import->importLoc.isValid() && sourceFile &&
847+
!sourceFile->hasImportUsedPreconcurrency(*import)) {
848+
SourceLoc importLoc = import->importLoc;
849+
ASTContext &ctx = nominal->getASTContext();
838850

839-
ctx.Diags.diagnose(
840-
importLoc, diag::add_predates_concurrency_import,
841-
ctx.LangOpts.isSwiftVersionAtLeast(6),
842-
nominal->getParentModule()->getName())
843-
.fixItInsert(importLoc, "@preconcurrency ");
851+
ctx.Diags
852+
.diagnose(importLoc, diag::add_predates_concurrency_import,
853+
ctx.LangOpts.isSwiftVersionAtLeast(6),
854+
nominal->getParentModule()->getName())
855+
.fixItInsert(importLoc, "@preconcurrency ");
844856

845-
sourceFile->setImportUsedPreconcurrency(*import);
857+
sourceFile->setImportUsedPreconcurrency(*import);
858+
}
846859
}
847860
}
848861

849862
return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
850863
}
851864

865+
void swift::diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf) {
866+
if (!shouldDiagnosePreconcurrencyImports(sf))
867+
return;
868+
869+
ASTContext &ctx = sf.getASTContext();
870+
871+
if (ctx.TypeCheckerOpts.SkipFunctionBodies != FunctionBodySkipping::None)
872+
return;
873+
874+
for (const auto &import : sf.getImports()) {
875+
if (import.options.contains(ImportFlags::Preconcurrency) &&
876+
import.importLoc.isValid() &&
877+
!sf.hasImportUsedPreconcurrency(import)) {
878+
ctx.Diags.diagnose(
879+
import.importLoc, diag::remove_predates_concurrency_import,
880+
import.module.importedModule->getName())
881+
.fixItRemove(import.preconcurrencyRange);
882+
}
883+
}
884+
}
885+
852886
/// Produce a diagnostic for a single instance of a non-Sendable type where
853887
/// a Sendable type is required.
854888
static bool diagnoseSingleNonSendableType(

lib/Sema/TypeCheckConcurrency.h

+5
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ bool diagnoseSendabilityErrorBasedOn(
399399
NominalTypeDecl *nominal, SendableCheckContext fromContext,
400400
llvm::function_ref<bool(DiagnosticBehavior)> diagnose);
401401

402+
/// If any of the imports in this source file was @preconcurrency but
403+
/// there were no diagnostics downgraded or suppressed due to that
404+
/// @preconcurrency, suggest that the attribute be removed.
405+
void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);
406+
402407
/// Given a set of custom attributes, pick out the global actor attributes
403408
/// and perform any necessary resolution and diagnostics, returning the
404409
/// global actor attribute and type it refers to (or \c None).

lib/Sema/TypeChecker.cpp

+1-32
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "swift/Subsystems.h"
1919
#include "TypeChecker.h"
20+
#include "TypeCheckConcurrency.h"
2021
#include "TypeCheckDecl.h"
2122
#include "TypeCheckObjC.h"
2223
#include "TypeCheckType.h"
@@ -258,38 +259,6 @@ void swift::performTypeChecking(SourceFile &SF) {
258259
TypeCheckSourceFileRequest{&SF}, {});
259260
}
260261

261-
/// If any of the imports in this source file was @preconcurrency but
262-
/// there were no diagnostics downgraded or suppressed due to that
263-
/// @preconcurrency, suggest that the attribute be removed.
264-
static void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf) {
265-
switch (sf.Kind) {
266-
case SourceFileKind::Interface:
267-
case SourceFileKind::SIL:
268-
return;
269-
270-
case SourceFileKind::Library:
271-
case SourceFileKind::Main:
272-
case SourceFileKind::MacroExpansion:
273-
break;
274-
}
275-
276-
ASTContext &ctx = sf.getASTContext();
277-
278-
if (ctx.TypeCheckerOpts.SkipFunctionBodies != FunctionBodySkipping::None)
279-
return;
280-
281-
for (const auto &import : sf.getImports()) {
282-
if (import.options.contains(ImportFlags::Preconcurrency) &&
283-
import.importLoc.isValid() &&
284-
!sf.hasImportUsedPreconcurrency(import)) {
285-
ctx.Diags.diagnose(
286-
import.importLoc, diag::remove_predates_concurrency_import,
287-
import.module.importedModule->getName())
288-
.fixItRemove(import.preconcurrencyRange);
289-
}
290-
}
291-
}
292-
293262
evaluator::SideEffect
294263
TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
295264
assert(SF && "Source file cannot be null!");

test/ModuleInterface/SilencePreconcurrency.swiftinterface

+8-1
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,15 @@
22
// swift-module-flags: -module-name SilencePreconcurrency
33

44
// RUN: %empty-directory(%t)
5-
// RUN: %target-swift-frontend -compile-module-from-interface -o %/t/SilencePreconcurrency.swiftmodule %s -verify
5+
// RUN: %target-swift-frontend -emit-module -o %t/Preconcurrency.swiftmodule -module-name Preconcurrency %S/Inputs/preconcurrency.swift
6+
// RUN: %target-swift-frontend -compile-module-from-interface -o %t/SilencePreconcurrency.swiftmodule %s -I %t -verify
67

78
// REQUIRES: OS=macosx
89

910
@preconcurrency import Swift
11+
import Preconcurrency
12+
13+
@available(macOS 10.15, *)
14+
public enum SendableEnum: Sendable {
15+
case caseWithNonSendablePayload(_ ns: NotSendable)
16+
}

0 commit comments

Comments
 (0)