Skip to content

Commit 64f6182

Browse files
committed
prevent double @MainActor annotation in ClangImporter
This can lead to latent type errors for API users, because a swiftmodule would otherwise be emitted, without any diagnostics, containing imported decl with two global actor annotations on it. Such decls will always be an error to the typechecker when its eventually encountered. This patch drops all `@MainActor` annotations after the first one in the ClangImporter, regardless of whether its the safe or unsafe version, and emits a warning when doing so.
1 parent cb6b1ea commit 64f6182

File tree

6 files changed

+56
-1
lines changed

6 files changed

+56
-1
lines changed

include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,9 @@ WARNING(implicit_bridging_header_imported_from_module,none,
8383
"is deprecated and will be removed in a later version of Swift",
8484
(StringRef, Identifier))
8585

86+
WARNING(import_multiple_mainactor_attr,none,
87+
"this attribute for global actor '%0' is invalid; the declaration already has attribute for global actor '%1'",
88+
(StringRef, StringRef))
89+
8690
#define UNDEFINE_DIAGNOSTIC_MACROS
8791
#include "DefineDiagnosticMacros.h"

lib/ClangImporter/ImportDecl.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8353,6 +8353,7 @@ void ClangImporter::Implementation::importAttributes(
83538353
// Scan through Clang attributes and map them onto Swift
83548354
// equivalents.
83558355
PatternBindingInitializer *initContext = nullptr;
8356+
Optional<const clang::SwiftAttrAttr *> SeenMainActorAttr;
83568357
bool AnyUnavailable = MappedDecl->getAttrs().isUnavailable(C);
83578358
for (clang::NamedDecl::attr_iterator AI = ClangDecl->attr_begin(),
83588359
AE = ClangDecl->attr_end(); AI != AE; ++AI) {
@@ -8496,15 +8497,32 @@ void ClangImporter::Implementation::importAttributes(
84968497
// __attribute__((swift_attr("attribute")))
84978498
//
84988499
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(*AI)) {
8499-
// FIXME: Hard-core @MainActor and @UIActor, because we don't have a
8500+
// FIXME: Hard-code @MainActor and @UIActor, because we don't have a
85008501
// point at which to do name lookup for imported entities.
85018502
if (auto isMainActor = isMainActorAttr(SwiftContext, swiftAttr)) {
85028503
bool isUnsafe = *isMainActor;
8504+
8505+
if (SeenMainActorAttr) {
8506+
// Cannot add main actor annotation twice. We'll keep the first
8507+
// one and raise a warning about the duplicate.
8508+
auto &clangSrcMgr = getClangASTContext().getSourceManager();
8509+
ClangSourceBufferImporter &bufferImporter =
8510+
getBufferImporterForDiagnostics();
8511+
SourceLoc attrLoc = bufferImporter.resolveSourceLocation(
8512+
clangSrcMgr, swiftAttr->getLocation());
8513+
8514+
diagnose(attrLoc, diag::import_multiple_mainactor_attr,
8515+
swiftAttr->getAttribute(),
8516+
SeenMainActorAttr.getValue()->getAttribute());
8517+
continue;
8518+
}
8519+
85038520
if (Type mainActorType = SwiftContext.getMainActorType()) {
85048521
auto typeExpr = TypeExpr::createImplicit(mainActorType, SwiftContext);
85058522
auto attr = CustomAttr::create(SwiftContext, SourceLoc(), typeExpr);
85068523
attr->setArgIsUnsafe(isUnsafe);
85078524
MappedDecl->getAttrs().add(attr);
8525+
SeenMainActorAttr = swiftAttr;
85088526
}
85098527

85108528
continue;
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
@import Foundation;
2+
3+
#pragma clang assume_nonnull begin
4+
5+
#define SWIFT_MAIN_ACTOR __attribute__((swift_attr("@MainActor")))
6+
#define SWIFT_UI_ACTOR __attribute__((swift_attr("@UIActor")))
7+
8+
// NOTE: If you ever end up removing support for the "@UIActor" alias,
9+
// just change both to be @MainActor and it won't change the purpose of
10+
// this test.
11+
12+
SWIFT_UI_ACTOR SWIFT_MAIN_ACTOR @protocol DoubleMainActor
13+
@required
14+
- (NSString *)createSeaShanty:(NSInteger)number;
15+
@end
16+
17+
#pragma clang assume_nonnull end
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-experimental-concurrency -import-objc-header %S/Inputs/DoubleMainActor.h -emit-module -module-name use %s 2> %t/stderr.txt
3+
// RUN: %FileCheck -input-file %t/stderr.txt %s
4+
5+
// REQUIRES: concurrency
6+
// REQUIRES: objc_interop
7+
8+
// CHECK: DoubleMainActor.h:{{[0-9]+}}:{{[0-9]+}}: warning: this attribute for global actor '@MainActor' is invalid; the declaration already has attribute for global actor '@UIActor'
9+
10+
protocol P : DoubleMainActor {}

test/IDE/print_clang_objc_async.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,5 @@ import _Concurrency
9292
// CHECK: {{^}}var MAGIC_NUMBER: Int32 { get }
9393

9494
// CHECK: func doSomethingConcurrently(_ block: @Sendable () -> Void)
95+
96+
// CHECK: @MainActor protocol TripleMainActor {

test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,4 +157,8 @@ void doSomethingConcurrently(__attribute__((noescape)) __attribute__((swift_attr
157157

158158
void doSomethingConcurrentlyButUnsafe(__attribute__((noescape)) __attribute__((swift_attr("@_unsafeSendable"))) void (^block)(void));
159159

160+
161+
MAIN_ACTOR MAIN_ACTOR __attribute__((__swift_attr__("@MainActor(unsafe)"))) @protocol TripleMainActor
162+
@end
163+
160164
#pragma clang assume_nonnull end

0 commit comments

Comments
 (0)