Skip to content

Commit 85977c3

Browse files
committed
[region-isolation] When RegionIsolation is enabled, delay the preconcurrency import not used diagnostic to the SIL pipeline.
The reason why I am doing this is that I am going to be adding support for preconcurrency imports to TransferNonSendable. That implies that we can have preconcurrency import suppression in the SIL pipeline and thus that emitting the diagnostic in Sema is too early. To do this, I introduced a new module pass called DiagnoseUnnecessaryPreconcurrencyImports that runs after the SILFunction pass TransferNonSendable. The reason why I use a module pass is to ensure that TransferNonSendable has run on all functions before we attempt to emit these diagnostics. Then in that pass, we iterate over all of the modules functions and construct a uniqued array of SourceFiles for these functions. Then we iterate over the uniqued SourceFiles and use the already constructed Sema machinery to emit the diagnostic using the source files. rdar://126928265 (cherry picked from commit c4f7076)
1 parent e857b46 commit 85977c3

File tree

7 files changed

+127
-7
lines changed

7 files changed

+127
-7
lines changed

include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,8 @@ PASS(ReferenceBindingTransform, "sil-reference-binding-transform",
501501
"Check/transform reference bindings")
502502
PASS(DiagnosticDeadFunctionElimination, "sil-diagnostic-dead-function-elim",
503503
"Eliminate dead functions from early specialization optimizations before we run later diagnostics")
504+
PASS(DiagnoseUnnecessaryPreconcurrencyImports, "sil-diagnose-unnecessary-preconcurrency-imports",
505+
"Diagnose any preconcurrency imports that Sema and TransferNonSendable did not use")
504506
PASS(PruneVTables, "prune-vtables",
505507
"Mark class methods that do not require vtable dispatch")
506508
PASS_RANGE(AllPasses, AADumper, PruneVTables)

include/swift/Sema/Concurrency.h

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//===--- Concurrency.h ----------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
///
13+
/// \file
14+
///
15+
/// This file defines concurrency utility routines from Sema that are used by
16+
/// later parts of the compiler like the pass pipeline.
17+
///
18+
//===----------------------------------------------------------------------===//
19+
20+
#ifndef SWIFT_SEMA_CONCURRENCY_H
21+
#define SWIFT_SEMA_CONCURRENCY_H
22+
23+
namespace swift {
24+
25+
class SourceFile;
26+
27+
/// If any of the imports in this source file was @preconcurrency but there were
28+
/// no diagnostics downgraded or suppressed due to that @preconcurrency, suggest
29+
/// that the attribute be removed.
30+
void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);
31+
32+
} // namespace swift
33+
34+
#endif

lib/SILOptimizer/Mandatory/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ target_sources(swiftSILOptimizer PRIVATE
99
ConsumeOperatorCopyableValuesChecker.cpp
1010
PhiStorageOptimizer.cpp
1111
ConstantPropagation.cpp
12+
DiagnoseUnnecessaryPreconcurrencyImports.cpp
1213
DebugInfoCanonicalizer.cpp
1314
DefiniteInitialization.cpp
1415
DIMemoryUseCollector.cpp
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
//===--- DiagnoseUnnecessaryPreconcurrencyImports.cpp ---------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
///
13+
/// \file
14+
///
15+
/// This is run after TransferNonSendable and uses Sema infrastructure to
16+
/// determine if in Sema or TransferNonSendable any of the preconcurrency import
17+
/// statements were not used.
18+
///
19+
/// This only runs when RegionIsolation is enabled. If RegionIsolation is
20+
/// disabled, we emit the unnecessary preconcurrency imports earlier during Sema
21+
/// since no later diagnostics will be emitted.
22+
///
23+
/// NOTE: This needs to be a module pass and run after TransferNonSendable so we
24+
/// can guarantee that we have run TransferNonSendable on all functions in our
25+
/// module before this runs.
26+
///
27+
//===----------------------------------------------------------------------===//
28+
29+
#include "swift/AST/SourceFile.h"
30+
#include "swift/SILOptimizer/PassManager/Transforms.h"
31+
#include "swift/Sema/Concurrency.h"
32+
33+
using namespace swift;
34+
35+
//===----------------------------------------------------------------------===//
36+
// MARK: Top Level Entrypoint
37+
//===----------------------------------------------------------------------===//
38+
39+
namespace {
40+
41+
class DiagnoseUnnecessaryPreconcurrencyImports : public SILModuleTransform {
42+
void run() override {
43+
// If region isolation is not enabled... return early.
44+
if (!getModule()->getASTContext().LangOpts.hasFeature(
45+
Feature::RegionBasedIsolation))
46+
return;
47+
48+
std::vector<SourceFile *> data;
49+
for (auto &fn : *getModule()) {
50+
auto *sf = fn.getSourceFile();
51+
if (!sf) {
52+
continue;
53+
}
54+
55+
data.push_back(sf);
56+
assert(sf->getBufferID() != -1 && "Must have a buffer id");
57+
}
58+
59+
// Sort unique by filename so our diagnostics are deterministic.
60+
//
61+
// TODO: If we cannot rely upon this, just sort by pointer address. Non
62+
// determinism emission of diagnostics isn't great but it isn't fatal.
63+
sortUnique(data, [](SourceFile *lhs, SourceFile *rhs) -> bool {
64+
return lhs->getBufferID() < rhs->getBufferID();
65+
});
66+
67+
// At this point, we know that we have our sorted unique list of source
68+
// files.
69+
for (auto *sf : data) {
70+
diagnoseUnnecessaryPreconcurrencyImports(*sf);
71+
}
72+
}
73+
};
74+
75+
} // namespace
76+
77+
SILTransform *swift::createDiagnoseUnnecessaryPreconcurrencyImports() {
78+
return new DiagnoseUnnecessaryPreconcurrencyImports();
79+
}

lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,11 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
140140
P.addTransferNonSendable();
141141

142142
// Now that we have completed running passes that use region analysis, clear
143-
// region analysis.
143+
// region analysis and emit diagnostics for unnecessary preconcurrency
144+
// imports.
144145
P.addRegionAnalysisInvalidationTransform();
146+
P.addDiagnoseUnnecessaryPreconcurrencyImports();
147+
145148
// Lower tuple addr constructor. Eventually this can be merged into later
146149
// passes. This ensures we do not need to update later passes for something
147150
// that is only needed by TransferNonSendable().

lib/Sema/TypeCheckConcurrency.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#include "swift/AST/Expr.h"
2424
#include "swift/AST/Module.h"
2525
#include "swift/AST/Type.h"
26+
#include "swift/Sema/Concurrency.h"
27+
2628
#include <cassert>
2729

2830
namespace swift {
@@ -487,11 +489,6 @@ bool diagnoseSendabilityErrorBasedOn(
487489
NominalTypeDecl *nominal, SendableCheckContext fromContext,
488490
llvm::function_ref<bool(DiagnosticBehavior)> diagnose);
489491

490-
/// If any of the imports in this source file was @preconcurrency but
491-
/// there were no diagnostics downgraded or suppressed due to that
492-
/// @preconcurrency, suggest that the attribute be removed.
493-
void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);
494-
495492
/// Given a set of custom attributes, pick out the global actor attributes
496493
/// and perform any necessary resolution and diagnostics, returning the
497494
/// global actor attribute and type it refers to (or \c std::nullopt).

lib/Sema/TypeChecker.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,11 @@ TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
315315
SF->typeCheckDelayedFunctions();
316316
}
317317

318-
diagnoseUnnecessaryPreconcurrencyImports(*SF);
318+
// If region based isolation is enabled, we diagnose unnecessary
319+
// preconcurrency imports in the SIL pipeline in the
320+
// DiagnoseUnnecessaryPreconcurrencyImports pass.
321+
if (!Ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation))
322+
diagnoseUnnecessaryPreconcurrencyImports(*SF);
319323
diagnoseUnnecessaryPublicImports(*SF);
320324

321325
// Check to see if there are any inconsistent imports.

0 commit comments

Comments
 (0)