Skip to content

Commit d283ffa

Browse files
tshortlitshortli
committed
AST: Add appropriate attributes to module import fix-its.
Make sure suggested imports are consistent with imports in other files. Also, make sure the underlying clang module is always imported `@_exported`.
1 parent f99a793 commit d283ffa

File tree

8 files changed

+115
-34
lines changed

8 files changed

+115
-34
lines changed

‎include/swift/AST/SourceFile.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,9 @@ class SourceFile final : public FileUnit {
457457
/// If not, we can fast-path module checks.
458458
bool hasImportsWithFlag(ImportFlags flag) const;
459459

460+
/// Returns the import flags that are active on imports of \p module.
461+
ImportFlags getImportFlags(const ModuleDecl *module) const;
462+
460463
/// Get the most permissive restriction applied to the imports of \p module.
461464
RestrictedImportKind getRestrictedImportKind(const ModuleDecl *module) const;
462465

‎lib/AST/Module.cpp

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2702,6 +2702,15 @@ bool SourceFile::hasImportsWithFlag(ImportFlags flag) const {
27022702
ctx.evaluator, HasImportsMatchingFlagRequest{mutableThis, flag}, false);
27032703
}
27042704

2705+
ImportFlags SourceFile::getImportFlags(const ModuleDecl *module) const {
2706+
unsigned flags = 0x0;
2707+
for (auto import : *Imports) {
2708+
if (import.module.importedModule == module)
2709+
flags |= import.options.toRaw();
2710+
}
2711+
return ImportFlags(flags);
2712+
}
2713+
27052714
bool SourceFile::hasTestableOrPrivateImport(
27062715
AccessLevel accessLevel, const swift::ValueDecl *ofDecl,
27072716
SourceFile::ImportQueryKind queryKind) const {
@@ -4016,25 +4025,47 @@ bool swift::diagnoseMissingImportForMember(const ValueDecl *decl,
40164025

40174026
SourceLoc bestLoc =
40184027
ctx.Diags.getBestAddImportFixItLoc(decl, dc->getParentSourceFile());
4019-
if (bestLoc.isValid()) {
4020-
llvm::SmallString<64> importText;
4021-
4022-
// @_spi imports.
4023-
if (decl->isSPI()) {
4024-
auto spiGroups = decl->getSPIGroups();
4025-
if (!spiGroups.empty()) {
4026-
importText += "@_spi(";
4027-
importText += spiGroups[0].str();
4028-
importText += ") ";
4029-
}
4030-
}
4028+
if (!bestLoc.isValid())
4029+
return false;
40314030

4032-
importText += "import ";
4033-
importText += definingModule->getName().str();
4034-
importText += "\n";
4035-
ctx.Diags.diagnose(bestLoc, diag::candidate_add_import, definingModule)
4036-
.fixItInsert(bestLoc, importText);
4031+
llvm::SmallString<64> importText;
4032+
4033+
// Check other source files for import flags that should be applied to the
4034+
// fix-it for consistency with the rest of the imports in the module.
4035+
auto parentModule = dc->getParentModule();
4036+
OptionSet<ImportFlags> flags;
4037+
for (auto file : parentModule->getFiles()) {
4038+
if (auto sf = dyn_cast<SourceFile>(file))
4039+
flags |= sf->getImportFlags(definingModule);
40374040
}
40384041

4042+
if (flags.contains(ImportFlags::Exported) ||
4043+
parentModule->isClangOverlayOf(definingModule))
4044+
importText += "@_exported ";
4045+
if (flags.contains(ImportFlags::ImplementationOnly))
4046+
importText += "@_implementationOnly ";
4047+
if (flags.contains(ImportFlags::WeakLinked))
4048+
importText += "@_weakLinked ";
4049+
if (flags.contains(ImportFlags::SPIOnly))
4050+
importText += "@_spiOnly ";
4051+
4052+
// FIXME: Access level should be considered, too.
4053+
4054+
// @_spi imports.
4055+
if (decl->isSPI()) {
4056+
auto spiGroups = decl->getSPIGroups();
4057+
if (!spiGroups.empty()) {
4058+
importText += "@_spi(";
4059+
importText += spiGroups[0].str();
4060+
importText += ") ";
4061+
}
4062+
}
4063+
4064+
importText += "import ";
4065+
importText += definingModule->getName().str();
4066+
importText += "\n";
4067+
ctx.Diags.diagnose(bestLoc, diag::candidate_add_import, definingModule)
4068+
.fixItInsert(bestLoc, importText);
4069+
40394070
return true;
40404071
}

‎test/NameLookup/Inputs/MemberImportVisibility/Underlying.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
struct UnderlyingStruct {
3+
int a;
4+
};

‎test/NameLookup/Inputs/MemberImportVisibility/members_transitive_other.swift

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎test/NameLookup/Inputs/MemberImportVisibility/module.modulemap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,7 @@ module Categories_D {
2323
}
2424
}
2525

26+
module Underlying {
27+
header "Underlying.h"
28+
export *
29+
}

‎test/NameLookup/members_transitive.swift

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/MemberImportVisibility/members_A.swift
33
// RUN: %target-swift-frontend -emit-module -I %t -o %t %S/Inputs/MemberImportVisibility/members_B.swift
44
// RUN: %target-swift-frontend -emit-module -I %t -o %t %S/Inputs/MemberImportVisibility/members_C.swift
5-
// RUN: %target-swift-frontend -typecheck -primary-file %s %S/Inputs/MemberImportVisibility/members_transitive_other.swift -I %t -verify -swift-version 5
6-
// RUN: %target-swift-frontend -typecheck -primary-file %s %S/Inputs/MemberImportVisibility/members_transitive_other.swift -I %t -verify -swift-version 6
7-
// RUN: %target-swift-frontend -typecheck -primary-file %s %S/Inputs/MemberImportVisibility/members_transitive_other.swift -I %t -verify -swift-version 5 -enable-experimental-feature MemberImportVisibility -verify-additional-prefix member-visibility-
5+
// RUN: %target-swift-frontend -typecheck %s -I %t -verify -swift-version 5
6+
// RUN: %target-swift-frontend -typecheck %s -I %t -verify -swift-version 6
7+
// RUN: %target-swift-frontend -typecheck %s -I %t -verify -swift-version 5 -enable-experimental-feature MemberImportVisibility -verify-additional-prefix member-visibility-
88

99
import members_C
10-
// expected-member-visibility-note 7{{add import of module 'members_B'}}{{1-1=import members_B\n}}
10+
// expected-member-visibility-note 6{{add import of module 'members_B'}}{{1-1=import members_B\n}}
1111

1212

1313
func testExtensionMembers(x: X, y: Y<Z>) {
@@ -32,12 +32,6 @@ func testOperatorMembers(x: X, y: Y<Z>) {
3232
_ = y <> y
3333
}
3434

35-
func testMembersWithContextualBase() {
36-
takesEnumInA(.caseInA)
37-
takesEnumInB(.caseInB) // expected-member-visibility-error{{enum case 'caseInB' is not available due to missing import of defining module 'members_B'}}
38-
takesEnumInC(.caseInC)
39-
}
40-
4135
extension X {
4236
var testProperties: (Bool, Bool, Bool) {
4337
return (

‎test/NameLookup/members_transitive_multifile.swift

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/MemberImportVisibility/members_A.swift
4+
// RUN: %target-swift-frontend -emit-module -I %t -o %t %S/Inputs/MemberImportVisibility/members_B.swift
5+
// RUN: %target-swift-frontend -emit-module -I %t -o %t %S/Inputs/MemberImportVisibility/members_C.swift
6+
// RUN: %target-swift-frontend -typecheck -primary-file %t/main.swift %t/A.swift %t/B.swift %t/C.swift -I %t -verify -swift-version 5
7+
// RUN: %target-swift-frontend -typecheck -primary-file %t/main.swift %t/A.swift %t/B.swift %t/C.swift -I %t -verify -swift-version 6
8+
// RUN: %target-swift-frontend -typecheck -primary-file %t/main.swift %t/A.swift %t/B.swift %t/C.swift -I %t -verify -swift-version 5 -enable-experimental-feature MemberImportVisibility -verify-additional-prefix member-visibility-
9+
10+
//--- main.swift
11+
12+
// expected-member-visibility-note@+3 {{add import of module 'members_A'}}{{1-1=@_implementationOnly import members_A\n}}
13+
// expected-member-visibility-note@+2 {{add import of module 'members_B'}}{{1-1=@_exported import members_B\n}}
14+
// expected-member-visibility-note@+1 {{add import of module 'members_C'}}{{1-1=@_weakLinked @_spiOnly import members_C\n}}
15+
func testMembersWithContextualBase() {
16+
takesEnumInA(.caseInA) // expected-member-visibility-error{{enum case 'caseInA' is not available due to missing import of defining module 'members_A'}}
17+
takesEnumInB(.caseInB) // expected-member-visibility-error{{enum case 'caseInB' is not available due to missing import of defining module 'members_B'}}
18+
takesEnumInC(.caseInC) // expected-member-visibility-error{{enum case 'caseInC' is not available due to missing import of defining module 'members_C'}}
19+
}
20+
21+
//--- A.swift
22+
23+
@_implementationOnly import members_A
24+
25+
func takesEnumInA(_ e: EnumInA) {}
26+
27+
//--- B.swift
28+
29+
@_exported import members_B
30+
31+
func takesEnumInB(_ e: EnumInB) {}
32+
33+
//--- C.swift
34+
35+
@_spiOnly @_weakLinked import members_C
36+
37+
func takesEnumInC(_ e: EnumInC) {}

‎test/NameLookup/members_transitive_underlying_clang.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 5
4+
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 6
5+
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 5 -enable-experimental-feature MemberImportVisibility -verify-additional-prefix member-visibility-
6+
7+
//--- Other.swift
8+
@_exported import Underlying
9+
10+
//--- Primary.swift
11+
12+
// expected-member-visibility-note@+1 {{add import of module 'Underlying'}}{{1-1=@_exported import Underlying\n}}
13+
func test(_ s: UnderlyingStruct) {
14+
_ = s.a // expected-member-visibility-error {{property 'a' is not available due to missing import of defining module 'Underlying'}}
15+
}

0 commit comments

Comments
 (0)