Skip to content

Commit 3c5fba3

Browse files
committed
When sorting imports for uniquing purposes, use full module names
Fixes a longstanding issue where submodules with the same name in different top-level modules weren't being sorted deterministically. This doesn't come up very much in practice, and it would have been hard to notice anything wrong, but it's good to be right.
1 parent 37ec248 commit 3c5fba3

File tree

3 files changed

+34
-3
lines changed

3 files changed

+34
-3
lines changed

lib/AST/Module.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,9 +1059,11 @@ ModuleDecl::removeDuplicateImports(SmallVectorImpl<ImportedModule> &imports) {
10591059
std::sort(imports.begin(), imports.end(),
10601060
[](const ImportedModule &lhs, const ImportedModule &rhs) -> bool {
10611061
// Arbitrarily sort by name to get a deterministic order.
1062-
// FIXME: Submodules don't get sorted properly here.
1063-
if (lhs.second != rhs.second)
1064-
return lhs.second->getName().str() < rhs.second->getName().str();
1062+
if (lhs.second != rhs.second) {
1063+
return std::lexicographical_compare(
1064+
lhs.second->getReverseFullModuleName(), {},
1065+
rhs.second->getReverseFullModuleName(), {});
1066+
}
10651067
using AccessPathElem = std::pair<Identifier, SourceLoc>;
10661068
return std::lexicographical_compare(lhs.first.begin(), lhs.first.end(),
10671069
rhs.first.begin(), rhs.first.end(),
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module X {
2+
explicit module Submodule {}
3+
}
4+
5+
module Y {
6+
explicit module Submodule {}
7+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %target-swift-frontend -emit-interface-path - -emit-module -o /dev/null %s -I %S/Inputs/imports-submodule-order/ | %FileCheck %s
2+
// RUN: %target-swift-frontend -emit-interface-path - -emit-module -o /dev/null %s -I %S/Inputs/imports-submodule-order/ -D XY | %FileCheck %s
3+
4+
#if XY
5+
@_exported import X.Submodule
6+
@_exported import Y.Submodule
7+
8+
#else
9+
@_exported import Y.Submodule
10+
@_exported import X.Submodule
11+
12+
#endif
13+
14+
// The order below is not alphabetical, just deterministic given a set of
15+
// imports across any number of files and in any order.
16+
17+
// CHECK-NOT: import
18+
// CHECK: import X.Submodule{{$}}
19+
// CHECK-NEXT: import Y.Submodule{{$}}
20+
// CHECK-NEXT: import X{{$}}
21+
// CHECK-NEXT: import Y{{$}}
22+
// CHECK-NOT: import

0 commit comments

Comments
 (0)