Skip to content

Commit c1444de

Browse files
kitaisrealkitaisreal
committed
SR-11889: Fixed code review issues
1 parent b7cb3b6 commit c1444de

19 files changed

+73
-63
lines changed

‎include/swift/AST/ASTContext.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
#include "swift/AST/Types.h"
2626
#include "swift/AST/TypeAlignments.h"
2727
#include "swift/Basic/LangOptions.h"
28-
#include "swift/Basic/Malloc.h"
2928
#include "swift/Basic/Located.h"
29+
#include "swift/Basic/Malloc.h"
3030
#include "llvm/ADT/ArrayRef.h"
3131
#include "llvm/ADT/DenseMap.h"
3232
#include "llvm/ADT/IntrusiveRefCntPtr.h"

‎include/swift/AST/ModuleLoader.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919

2020
#include "swift/AST/Identifier.h"
2121
#include "swift/Basic/LLVM.h"
22+
#include "swift/Basic/Located.h"
2223
#include "swift/Basic/SourceLoc.h"
2324
#include "llvm/ADT/SetVector.h"
2425
#include "llvm/ADT/SmallSet.h"
2526
#include "llvm/ADT/TinyPtrVector.h"
26-
#include "swift/Basic/Located.h"
2727

2828
namespace llvm {
2929
class FileCollector;

‎include/swift/AST/NameLookup.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,15 +495,15 @@ void recordLookupOfTopLevelName(DeclContext *topLevelContext, DeclName name,
495495
void getDirectlyInheritedNominalTypeDecls(
496496
llvm::PointerUnion<TypeDecl *, ExtensionDecl *> decl,
497497
unsigned i,
498-
llvm::SmallVectorImpl<std::pair<SourceLoc, NominalTypeDecl *>> &result,
498+
llvm::SmallVectorImpl<Located<NominalTypeDecl *>> &result,
499499
bool &anyObject);
500500

501501
/// Retrieve the set of nominal type declarations that are directly
502502
/// "inherited" by the given declaration, looking through typealiases
503503
/// and splitting out the components of compositions.
504504
///
505505
/// If we come across the AnyObject type, set \c anyObject true.
506-
SmallVector<std::pair<SourceLoc, NominalTypeDecl *>, 4>
506+
SmallVector<Located<NominalTypeDecl *>, 4>
507507
getDirectlyInheritedNominalTypeDecls(
508508
llvm::PointerUnion<TypeDecl *, ExtensionDecl *> decl,
509509
bool &anyObject);

‎include/swift/AST/TypeRepr.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "llvm/ADT/PointerUnion.h"
2727
#include "llvm/ADT/STLExtras.h"
2828
#include "swift/Basic/Debug.h"
29+
#include "swift/Basic/Located.h"
2930
#include "swift/Basic/InlineBitfield.h"
3031
#include "llvm/Support/ErrorHandling.h"
3132
#include "llvm/Support/TrailingObjects.h"
@@ -672,9 +673,9 @@ struct TupleTypeReprElement {
672673
/// \endcode
673674
class TupleTypeRepr final : public TypeRepr,
674675
private llvm::TrailingObjects<TupleTypeRepr, TupleTypeReprElement,
675-
std::pair<SourceLoc, unsigned>> {
676+
Located<unsigned>> {
676677
friend TrailingObjects;
677-
typedef std::pair<SourceLoc, unsigned> SourceLocAndIdx;
678+
typedef Located<unsigned> SourceLocAndIdx;
678679

679680
SourceRange Parens;
680681

@@ -745,21 +746,21 @@ class TupleTypeRepr final : public TypeRepr,
745746

746747
SourceLoc getEllipsisLoc() const {
747748
return hasEllipsis() ?
748-
getTrailingObjects<SourceLocAndIdx>()[0].first : SourceLoc();
749+
getTrailingObjects<SourceLocAndIdx>()[0].loc : SourceLoc();
749750
}
750751

751752
unsigned getEllipsisIndex() const {
752753
return hasEllipsis() ?
753-
getTrailingObjects<SourceLocAndIdx>()[0].second :
754+
getTrailingObjects<SourceLocAndIdx>()[0].item :
754755
Bits.TupleTypeRepr.NumElements;
755756
}
756757

757758
void removeEllipsis() {
758759
if (hasEllipsis()) {
759760
Bits.TupleTypeRepr.HasEllipsis = false;
760761
getTrailingObjects<SourceLocAndIdx>()[0] = {
761-
SourceLoc(),
762-
getNumElements()
762+
getNumElements(),
763+
SourceLoc()
763764
};
764765
}
765766
}

‎include/swift/Basic/Located.h

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,15 @@
22
//
33
// This source file is part of the Swift.org open source project
44
//
5-
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
5+
// Copyright (c) 2019 Apple Inc. and the Swift project authors
66
// Licensed under Apache License v2.0 with Runtime Library Exception
77
//
88
// See https://swift.org/LICENSE.txt for license information
99
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
1010
//
1111
//===----------------------------------------------------------------------===//
1212
//
13-
// This file forward declares and imports various common LLVM datatypes that
14-
// swift wants to use unqualified.
13+
// Provides a currency data type Located<T> that should be used instead of std::pair<T, SourceLoc>.
1514
//
1615
//===----------------------------------------------------------------------===//
1716

@@ -22,15 +21,27 @@
2221

2322
namespace swift {
2423

24+
/// A currency type for keeping track of items which were found in the source code.
25+
///
26+
/// Several parts of the compiler need to keep track of a `SourceLoc` corresponding to an item, in case
27+
/// they need to report some diagnostics later. For example, the ClangImporter needs to keep track
28+
/// of where imports were originally written. Located makes it easy to do so while making the code
29+
/// more readable, compared to using `std::pair`.
2530
template<typename T>
2631
struct Located {
2732

33+
/// The main item whose source location is being tracked.
2834
T item;
2935

36+
/// The original source location from which the item was parsed.
3037
SourceLoc loc;
3138

39+
Located() {}
40+
41+
Located(T item, SourceLoc loc): item(item), loc(loc) {}
42+
3243
template<typename U>
33-
friend bool operator ==(const Located<U> lhs, const Located<U> rhs) {
44+
friend bool operator ==(const Located<U>& lhs, const Located<U>& rhs) {
3445
return lhs.item == rhs.item && lhs.loc == rhs.loc;
3546
}
3647
};

‎include/swift/IDE/Utils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ class NameMatcher: public ASTWalker {
240240

241241
/// The \c Expr argument of a parent \c CustomAttr (if one exists) and
242242
/// the \c SourceLoc of the type name it applies to.
243-
llvm::Optional<std::pair<SourceLoc, Expr *>> CustomAttrArg;
243+
llvm::Optional<Located<Expr *>> CustomAttrArg;
244244
unsigned InactiveConfigRegionNestings = 0;
245245
unsigned SelectorNestings = 0;
246246

‎include/swift/Parse/Parser.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class Parser {
120120
DeclContext *CurDeclContext;
121121
ASTContext &Context;
122122
CodeCompletionCallbacks *CodeCompletion = nullptr;
123-
std::vector<std::pair<SourceLoc, std::vector<ParamDecl*>>> AnonClosureVars;
123+
std::vector<Located<std::vector<ParamDecl*>>> AnonClosureVars;
124124

125125
bool IsParsingInterfaceTokens = false;
126126

‎lib/AST/ConformanceLookupTable.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ void ConformanceLookupTable::destroy() {
140140
}
141141

142142
namespace {
143-
using ConformanceConstructionInfo = std::pair<SourceLoc, ProtocolDecl *>;
143+
using ConformanceConstructionInfo = Located<ProtocolDecl *>;
144144
}
145145

146146
template<typename NominalFunc, typename ExtensionFunc>
@@ -194,14 +194,14 @@ void ConformanceLookupTable::forEachInStage(ConformanceStage stage,
194194
loader.first->loadAllConformances(next, loader.second, conformances);
195195
loadAllConformances(next, conformances);
196196
for (auto conf : conformances) {
197-
protocols.push_back({SourceLoc(), conf->getProtocol()});
197+
protocols.push_back({conf->getProtocol(), SourceLoc()});
198198
}
199199
} else if (next->getParentSourceFile()) {
200200
bool anyObject = false;
201201
for (const auto &found :
202202
getDirectlyInheritedNominalTypeDecls(next, anyObject)) {
203-
if (auto proto = dyn_cast<ProtocolDecl>(found.second))
204-
protocols.push_back({found.first, proto});
203+
if (auto proto = dyn_cast<ProtocolDecl>(found.item))
204+
protocols.push_back({proto, found.loc});
205205
}
206206
}
207207

@@ -281,7 +281,7 @@ void ConformanceLookupTable::updateLookupTable(NominalTypeDecl *nominal,
281281
// its inherited protocols directly.
282282
auto source = ConformanceSource::forExplicit(ext);
283283
for (auto locAndProto : protos)
284-
addProtocol(locAndProto.second, locAndProto.first, source);
284+
addProtocol(locAndProto.item, locAndProto.loc, source);
285285
});
286286
break;
287287

@@ -470,8 +470,8 @@ void ConformanceLookupTable::addInheritedProtocols(
470470
bool anyObject = false;
471471
for (const auto &found :
472472
getDirectlyInheritedNominalTypeDecls(decl, anyObject)) {
473-
if (auto proto = dyn_cast<ProtocolDecl>(found.second))
474-
addProtocol(proto, found.first, source);
473+
if (auto proto = dyn_cast<ProtocolDecl>(found.item))
474+
addProtocol(proto, found.loc, source);
475475
}
476476
}
477477

‎lib/AST/Decl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4538,7 +4538,7 @@ ProtocolDecl::getInheritedProtocolsSlow() {
45384538
for (const auto found :
45394539
getDirectlyInheritedNominalTypeDecls(
45404540
const_cast<ProtocolDecl *>(this), anyObject)) {
4541-
if (auto proto = dyn_cast<ProtocolDecl>(found.second)) {
4541+
if (auto proto = dyn_cast<ProtocolDecl>(found.item)) {
45424542
if (known.insert(proto).second)
45434543
result.push_back(proto);
45444544
}

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3939,13 +3939,13 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
39393939

39403940
// Local function to find the insertion point for the protocol's "where"
39413941
// clause, as well as the string to start the insertion ("where" or ",");
3942-
auto getProtocolWhereLoc = [&]() -> std::pair<SourceLoc, const char *> {
3942+
auto getProtocolWhereLoc = [&]() -> Located<const char *> {
39433943
// Already has a trailing where clause.
39443944
if (auto trailing = proto->getTrailingWhereClause())
3945-
return { trailing->getRequirements().back().getSourceRange().End, ", " };
3945+
return { ", ", trailing->getRequirements().back().getSourceRange().End };
39463946

39473947
// Inheritance clause.
3948-
return { proto->getInherited().back().getSourceRange().End, " where " };
3948+
return { " where ", proto->getInherited().back().getSourceRange().End };
39493949
};
39503950

39513951
// Retrieve the set of requirements that a given associated type declaration
@@ -4060,8 +4060,8 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
40604060
assocTypeDecl->getFullName(),
40614061
inheritedFromProto->getDeclaredInterfaceType())
40624062
.fixItInsertAfter(
4063-
fixItWhere.first,
4064-
getAssociatedTypeReqs(assocTypeDecl, fixItWhere.second))
4063+
fixItWhere.loc,
4064+
getAssociatedTypeReqs(assocTypeDecl, fixItWhere.item))
40654065
.fixItRemove(assocTypeDecl->getSourceRange());
40664066

40674067
Diags.diagnose(inheritedAssocTypeDecl, diag::decl_declared_here,
@@ -4136,8 +4136,8 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
41364136
diag::typealias_override_associated_type,
41374137
name,
41384138
inheritedFromProto->getDeclaredInterfaceType())
4139-
.fixItInsertAfter(fixItWhere.first,
4140-
getConcreteTypeReq(type, fixItWhere.second))
4139+
.fixItInsertAfter(fixItWhere.loc,
4140+
getConcreteTypeReq(type, fixItWhere.item))
41414141
.fixItRemove(type->getSourceRange());
41424142
Diags.diagnose(inheritedAssocTypeDecl, diag::decl_declared_here,
41434143
inheritedAssocTypeDecl->getFullName());

0 commit comments

Comments
 (0)