Skip to content

Commit 622e47c

Browse files
committed
[Sema] Report conflicting attributes on imports
1 parent ebc2162 commit 622e47c

File tree

3 files changed

+92
-11
lines changed

3 files changed

+92
-11
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -964,8 +964,9 @@ ERROR(module_not_testable,Fatal,
964964
ERROR(module_not_compiled_for_private_import,none,
965965
"module %0 was not compiled for private import", (Identifier))
966966

967-
ERROR(import_implementation_cannot_be_exported,none,
968-
"module %0 cannot be both exported and implementation-only", (Identifier))
967+
ERROR(import_restriction_conflict,none,
968+
"module %0 cannot be both %1 and %2",
969+
(Identifier, StringRef, StringRef))
969970

970971
WARNING(module_not_compiled_with_library_evolution,none,
971972
"module %0 was not compiled with library evolution support; "

lib/Sema/ImportResolution.cpp

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,11 @@ struct UnboundImport {
128128

129129
private:
130130
void validatePrivate(ModuleDecl *topLevelModule);
131-
void validateImplementationOnly(ASTContext &ctx);
131+
132+
/// Check that no import has more than one of the following modifiers:
133+
/// @_exported, @_implementationOnly, and @_spiOnly.
134+
void validateRestrictedImport(ASTContext &ctx);
135+
132136
void validateTestable(ModuleDecl *topLevelModule);
133137
void validateResilience(NullablePtr<ModuleDecl> topLevelModule,
134138
SourceFile &SF);
@@ -598,7 +602,7 @@ bool UnboundImport::checkModuleLoaded(ModuleDecl *M, SourceFile &SF) {
598602

599603
void UnboundImport::validateOptions(NullablePtr<ModuleDecl> topLevelModule,
600604
SourceFile &SF) {
601-
validateImplementationOnly(SF.getASTContext());
605+
validateRestrictedImport(SF.getASTContext());
602606

603607
if (auto *top = topLevelModule.getPtrOrNull()) {
604608
// FIXME: Having these two calls in this if condition seems dubious.
@@ -633,16 +637,55 @@ void UnboundImport::validatePrivate(ModuleDecl *topLevelModule) {
633637
import.sourceFileArg = StringRef();
634638
}
635639

636-
void UnboundImport::validateImplementationOnly(ASTContext &ctx) {
637-
if (!import.options.contains(ImportFlags::ImplementationOnly) ||
638-
!import.options.contains(ImportFlags::Exported))
640+
void UnboundImport::validateRestrictedImport(ASTContext &ctx) {
641+
static llvm::SmallVector<ImportFlags, 2> flags = {ImportFlags::Exported,
642+
ImportFlags::ImplementationOnly,
643+
ImportFlags::SPIOnly};
644+
llvm::SmallVector<ImportFlags, 2> conflicts;
645+
646+
for (auto flag : flags) {
647+
if (import.options.contains(flag))
648+
conflicts.push_back(flag);
649+
}
650+
651+
// Quit if there's no conflicting attributes.
652+
if (conflicts.size() < 2)
639653
return;
640654

641-
// Remove one flag to maintain the invariant.
642-
import.options -= ImportFlags::ImplementationOnly;
655+
// Remove all but one flag to maintain the invariant.
656+
for (auto iter = conflicts.begin(); iter != std::prev(conflicts.end()); iter ++)
657+
import.options -= *iter;
658+
659+
DeclAttrKind attrToRemove = conflicts[0] == ImportFlags::ImplementationOnly?
660+
DAK_Exported : DAK_ImplementationOnly;
661+
662+
auto flagName = [](ImportFlags flag) {
663+
switch (flag) {
664+
case ImportFlags::ImplementationOnly:
665+
return "implementation-only";
666+
case ImportFlags::SPIOnly:
667+
return "SPI only";
668+
case ImportFlags::Exported:
669+
return "exported";
670+
default:
671+
llvm_unreachable("Unexpected ImportFlag");
672+
}
673+
};
674+
675+
// Report the conflict, only the first two conflicts should be enough.
676+
auto diag = ctx.Diags.diagnose(import.module.getModulePath().front().Loc,
677+
diag::import_restriction_conflict,
678+
import.module.getModulePath().front().Item,
679+
flagName(conflicts[0]),
680+
flagName(conflicts[1]));
643681

644-
diagnoseInvalidAttr(DAK_ImplementationOnly, ctx.Diags,
645-
diag::import_implementation_cannot_be_exported);
682+
auto *ID = getImportDecl().getPtrOrNull();
683+
if (!ID) return;
684+
auto *attr = ID->getAttrs().getAttribute(attrToRemove);
685+
if (!attr) return;
686+
687+
diag.fixItRemove(attr->getRangeWithAt());
688+
attr->setInvalid();
646689
}
647690

648691
void UnboundImport::validateTestable(ModuleDecl *topLevelModule) {
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/// Test conflicting imports modifiers from the same line.
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: split-file %s %t
5+
6+
/// Generate dependencies.
7+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
8+
// RUN: -module-name Lib -emit-module-path %t/Lib.swiftmodule \
9+
// RUN: -swift-version 5 -enable-library-evolution
10+
11+
/// Build clients.
12+
// RUN: %target-swift-frontend -typecheck %t/SPIOnly_Exported.swift -I %t -verify \
13+
// RUN: -experimental-spi-only-imports -verify
14+
// RUN: %target-swift-frontend -typecheck %t/Exported_SPIOnly.swift -I %t -verify \
15+
// RUN: -experimental-spi-only-imports -verify
16+
// RUN: %target-swift-frontend -typecheck %t/SPIOnly_IOI.swift -I %t -verify \
17+
// RUN: -experimental-spi-only-imports -verify
18+
// RUN: %target-swift-frontend -typecheck %t/SPIOnly_IOI_Exported.swift -I %t -verify \
19+
// RUN: -experimental-spi-only-imports -verify
20+
21+
//--- Lib.swift
22+
// Empty source file for import.
23+
24+
//--- SPIOnly_Exported.swift
25+
@_spiOnly @_exported import Lib // expected-error {{module 'Lib' cannot be both exported and SPI only}}
26+
27+
//--- Exported_SPIOnly.swift
28+
@_exported @_spiOnly import Lib // expected-error {{module 'Lib' cannot be both exported and SPI only}}
29+
30+
//--- SPIOnly_IOI.swift
31+
@_spiOnly @_implementationOnly import Lib // expected-error {{module 'Lib' cannot be both implementation-only and SPI only}}
32+
33+
//--- Exported_IOI.swift
34+
@_exported @_implementationOnly import Lib // expected-error {{module 'Lib' cannot be both exported and implementation-only}}
35+
36+
//--- SPIOnly_IOI_Exported.swift
37+
@_spiOnly @_implementationOnly @_exported import Lib // expected-error {{module 'Lib' cannot be both exported and implementation-only}}

0 commit comments

Comments
 (0)