Skip to content

Commit f18f3da

Browse files
committed
Fix SR-13490: Fix LT operator and add tests for ImportPath
1 parent 163d47e commit f18f3da

File tree

4 files changed

+79
-11
lines changed

4 files changed

+79
-11
lines changed

Diff for: include/swift/AST/Import.h

+17-2
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@
2222
#include "swift/AST/Identifier.h"
2323
#include "swift/Basic/Located.h"
2424
#include "llvm/ADT/ArrayRef.h"
25-
#include "llvm/ADT/StringRef.h"
26-
#include "llvm/ADT/SmallVector.h"
2725
#include "llvm/ADT/STLExtras.h"
26+
#include "llvm/ADT/SmallVector.h"
27+
#include "llvm/ADT/StringRef.h"
28+
#include <algorithm>
2829

2930
namespace swift {
3031
class ASTContext;
@@ -191,6 +192,20 @@ namespace detail {
191192
};
192193
}
193194

195+
/// @name ImportPathBase Comparison Operators
196+
/// @{
197+
template <typename Subclass>
198+
inline bool operator<(const detail::ImportPathBase<Subclass> &LHS,
199+
const detail::ImportPathBase<Subclass> &RHS) {
200+
using Element = typename detail::ImportPathBase<Subclass>::Element;
201+
auto Comparator = [](const Element &l, const Element &r) {
202+
return l.Item.compare(r.Item) < 0;
203+
};
204+
return std::lexicographical_compare(LHS.begin(), LHS.end(), RHS.begin(),
205+
RHS.end(), Comparator);
206+
}
207+
/// @}
208+
194209
/// An undifferentiated series of dotted identifiers in an \c import statement,
195210
/// like \c Foo.Bar. Each identifier is packaged with its corresponding source
196211
/// location.

Diff for: lib/IDE/ModuleInterfacePrinting.cpp

+1-9
Original file line numberDiff line numberDiff line change
@@ -374,15 +374,7 @@ static bool printModuleInterfaceDecl(Decl *D,
374374

375375
/// Sorts import declarations for display.
376376
static bool compareImports(ImportDecl *LHS, ImportDecl *RHS) {
377-
// TODO(SR-13490): Probably buggy--thinks "import Foo" == "import Foo.Bar".
378-
// ImportPathBase should provide universal comparison functions to avoid this.
379-
auto LHSPath = LHS->getImportPath();
380-
auto RHSPath = RHS->getImportPath();
381-
for (unsigned i: range(std::min(LHSPath.size(), RHSPath.size()))) {
382-
if (int Ret = LHSPath[i].Item.str().compare(RHSPath[i].Item.str()))
383-
return Ret < 0;
384-
}
385-
return false;
377+
return LHS->getImportPath() < RHS->getImportPath();
386378
};
387379

388380
/// Sorts Swift declarations for display.

Diff for: unittests/AST/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ add_swift_unittest(SwiftASTTests
66
TestContext.cpp
77
TypeMatchTests.cpp
88
VersionRangeLattice.cpp
9+
ImportTests.cpp
910
)
1011

1112
target_link_libraries(SwiftASTTests

Diff for: unittests/AST/ImportTests.cpp

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//===--- ImportTests.cpp - Tests for representation of imports ------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2017 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+
#include "TestContext.h"
14+
#include "swift/AST/Import.h"
15+
#include "gtest/gtest.h"
16+
17+
using namespace swift;
18+
using namespace swift::unittest;
19+
20+
namespace {
21+
/// Helper class used to create ImportPath and hold all strings for identifiers
22+
class ImportPathContext {
23+
TestContext Ctx;
24+
25+
public:
26+
ImportPathContext() = default;
27+
28+
/// Hepler routine for building ImportPath
29+
/// Build()
30+
/// @see ImportPathBuilder
31+
inline ImportPath Build(StringRef Name) noexcept {
32+
return ImportPath::Builder(Ctx.Ctx, Name, '.').copyTo(Ctx.Ctx);
33+
}
34+
};
35+
36+
} // namespace
37+
38+
TEST(ImportPath, Comparison) {
39+
ImportPathContext ctx;
40+
41+
/// Simple sanity check:
42+
EXPECT_FALSE(ctx.Build("A.B.C") < ctx.Build("A.B.C"));
43+
44+
/// Check order chain:
45+
/// A < A.A < A.A.A < A.A.B < A.B < A.B.A < AA < B < B.A
46+
EXPECT_LT(ctx.Build("A"), ctx.Build("A.A"));
47+
EXPECT_LT(ctx.Build("A.A"), ctx.Build("A.A.A"));
48+
EXPECT_LT(ctx.Build("A.A.A"), ctx.Build("A.A.B"));
49+
EXPECT_LT(ctx.Build("A.A.B"), ctx.Build("A.B"));
50+
EXPECT_LT(ctx.Build("A.B"), ctx.Build("A.B.A"));
51+
EXPECT_LT(ctx.Build("A.B.A"), ctx.Build("AA"));
52+
EXPECT_LT(ctx.Build("B"), ctx.Build("B.A"));
53+
54+
/// Further ImportPaths are semantically incorrect, but we must
55+
/// check that comparing them does not cause compiler to crash.
56+
EXPECT_LT(ctx.Build("."), ctx.Build("A"));
57+
EXPECT_LT(ctx.Build("A"), ctx.Build("AA."));
58+
EXPECT_LT(ctx.Build("A"), ctx.Build("AA.."));
59+
EXPECT_LT(ctx.Build(".A"), ctx.Build("AA"));
60+
}

0 commit comments

Comments
 (0)