Skip to content

Commit 8857deb

Browse files
swift-ciswift-ci
authored
Merge pull request swiftlang#34095 from Interfere/SR-13490-fix-compareImports
2 parents 5e399b9 + 8615904 commit 8857deb

File tree

7 files changed

+96
-28
lines changed

7 files changed

+96
-28
lines changed

‎include/swift/AST/Import.h

Lines changed: 17 additions & 2 deletions
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.

‎lib/IDE/ModuleInterfacePrinting.cpp

Lines changed: 1 addition & 9 deletions
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.

‎test/IDE/print_ast_overlay.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,6 @@ public class FooOverlayClassDerived : FooOverlayClassBase {
4141

4242
// PASS_NO_INTERNAL-NOT: overlay_func_internal
4343

44-
// PASS_ANNOTATED: <decl:Import>@_exported import <ref:module>Foo</ref>.<ref:module>FooSub</ref></decl>
4544
// PASS_ANNOTATED: <decl:Import>@_exported import <ref:module>Foo</ref></decl>
45+
// PASS_ANNOTATED: <decl:Import>@_exported import <ref:module>Foo</ref>.<ref:module>FooSub</ref></decl>
4646
// PASS_ANNOTATED: <decl:Import>@_exported import <ref:module>FooHelper</ref></decl>

‎test/SourceKit/InterfaceGen/gen_clang_module.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,21 +73,21 @@ var x: FooClassBase
7373

7474
// RUN: %sourcekitd-test -req=interface-gen-open -module Foo -- -I %t.overlays -F %S/../Inputs/libIDE-mock-sdk \
7575
// RUN: -target %target-triple %clang-importer-sdk-nosource -I %t \
76-
// RUN: == -req=cursor -pos=1:8 == -req=cursor -pos=1:12 \
77-
// RUN: == -req=cursor -pos=2:10 \
78-
// RUN: == -req=cursor -pos=3:10 | %FileCheck -check-prefix=CHECK-IMPORT %s
76+
// RUN: == -req=cursor -pos=1:8 \
77+
// RUN: == -req=cursor -pos=2:8 == -req=cursor -pos=2:12 \
78+
// RUN: == -req=cursor -pos=3:8 | %FileCheck -check-prefix=CHECK-IMPORT %s
7979
// The cursors point to module names inside the imports, see 'gen_clang_module.swift.response'
8080

8181
// CHECK-IMPORT: source.lang.swift.ref.module ()
8282
// CHECK-IMPORT-NEXT: Foo{{$}}
8383
// CHECK-IMPORT-NEXT: Foo{{$}}
8484
// CHECK-IMPORT: source.lang.swift.ref.module ()
85-
// CHECK-IMPORT-NEXT: FooSub{{$}}
86-
// CHECK-IMPORT-NEXT: Foo.FooSub{{$}}
87-
// CHECK-IMPORT: source.lang.swift.ref.module ()
8885
// CHECK-IMPORT-NEXT: Foo{{$}}
8986
// CHECK-IMPORT-NEXT: Foo{{$}}
9087
// CHECK-IMPORT: source.lang.swift.ref.module ()
88+
// CHECK-IMPORT-NEXT: FooSub{{$}}
89+
// CHECK-IMPORT-NEXT: Foo.FooSub{{$}}
90+
// CHECK-IMPORT: source.lang.swift.ref.module ()
9191
// CHECK-IMPORT-NEXT: FooHelper{{$}}
9292
// CHECK-IMPORT-NEXT: FooHelper{{$}}
9393

‎test/SourceKit/InterfaceGen/gen_clang_module.swift.response

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import Foo.FooSub
21
import Foo
2+
import Foo.FooSub
33
import FooHelper
44
import SwiftOnoneSupport
55

@@ -370,19 +370,19 @@ public class FooOverlayClassDerived : Foo.FooOverlayClassBase {
370370
key.length: 3
371371
},
372372
{
373-
key.kind: source.lang.swift.syntaxtype.identifier,
373+
key.kind: source.lang.swift.syntaxtype.keyword,
374374
key.offset: 11,
375375
key.length: 6
376376
},
377377
{
378-
key.kind: source.lang.swift.syntaxtype.keyword,
378+
key.kind: source.lang.swift.syntaxtype.identifier,
379379
key.offset: 18,
380-
key.length: 6
380+
key.length: 3
381381
},
382382
{
383383
key.kind: source.lang.swift.syntaxtype.identifier,
384-
key.offset: 25,
385-
key.length: 3
384+
key.offset: 22,
385+
key.length: 6
386386
},
387387
{
388388
key.kind: source.lang.swift.syntaxtype.keyword,
@@ -3608,13 +3608,13 @@ public class FooOverlayClassDerived : Foo.FooOverlayClassBase {
36083608
},
36093609
{
36103610
key.kind: source.lang.swift.ref.module,
3611-
key.offset: 11,
3612-
key.length: 6
3611+
key.offset: 18,
3612+
key.length: 3
36133613
},
36143614
{
36153615
key.kind: source.lang.swift.ref.module,
3616-
key.offset: 25,
3617-
key.length: 3
3616+
key.offset: 22,
3617+
key.length: 6
36183618
},
36193619
{
36203620
key.kind: source.lang.swift.ref.module,

‎unittests/AST/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
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

‎unittests/AST/ImportTests.cpp

Lines changed: 60 additions & 0 deletions
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)