Skip to content

[Compiler]: Using Located<T> instead of std::pair<SourceLoc, T> #28643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
7 changes: 4 additions & 3 deletions include/swift/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "swift/AST/Types.h"
#include "swift/AST/TypeAlignments.h"
#include "swift/Basic/LangOptions.h"
#include "swift/Basic/Located.h"
#include "swift/Basic/Malloc.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
Expand Down Expand Up @@ -718,12 +719,12 @@ class ASTContext final {
///
/// Note that even if this check succeeds, errors may still occur if the
/// module is loaded in full.
bool canImportModule(std::pair<Identifier, SourceLoc> ModulePath);
bool canImportModule(Located<Identifier> ModulePath);

/// \returns a module with a given name that was already loaded. If the
/// module was not loaded, returns nullptr.
ModuleDecl *getLoadedModule(
ArrayRef<std::pair<Identifier, SourceLoc>> ModulePath) const;
ArrayRef<Located<Identifier>> ModulePath) const;

ModuleDecl *getLoadedModule(Identifier ModuleName) const;

Expand All @@ -733,7 +734,7 @@ class ASTContext final {
/// be returned.
///
/// \returns The requested module, or NULL if the module cannot be found.
ModuleDecl *getModule(ArrayRef<std::pair<Identifier, SourceLoc>> ModulePath);
ModuleDecl *getModule(ArrayRef<Located<Identifier>> ModulePath);

ModuleDecl *getModuleByName(StringRef ModuleName);

Expand Down
8 changes: 3 additions & 5 deletions include/swift/AST/Attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "swift/Basic/Range.h"
#include "swift/Basic/OptimizationMode.h"
#include "swift/Basic/Version.h"
#include "swift/Basic/Located.h"
#include "swift/AST/Identifier.h"
#include "swift/AST/AttrKind.h"
#include "swift/AST/AutoDiff.h"
Expand Down Expand Up @@ -69,16 +70,13 @@ class TypeAttributes {
struct Convention {
StringRef Name = {};
DeclNameRef WitnessMethodProtocol = {};
StringRef ClangType = {};
// Carry the source location for diagnostics.
SourceLoc ClangTypeLoc = {};

Located<StringRef> ClangType = Located<StringRef>(StringRef(), {});
/// Convenience factory function to create a Swift convention.
///
/// Don't use this function if you are creating a C convention as you
/// probably need a ClangType field as well.
static Convention makeSwiftConvention(StringRef name) {
return {name, DeclNameRef(), "", {}};
return {name, DeclNameRef(), Located<StringRef>("", {})};
}
};

Expand Down
9 changes: 5 additions & 4 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "swift/Basic/NullablePtr.h"
#include "swift/Basic/OptionalEnum.h"
#include "swift/Basic/Range.h"
#include "swift/Basic/Located.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/Support/TrailingObjects.h"
Expand Down Expand Up @@ -1503,11 +1504,11 @@ enum class ImportKind : uint8_t {
/// import Swift
/// import typealias Swift.Int
class ImportDecl final : public Decl,
private llvm::TrailingObjects<ImportDecl, std::pair<Identifier,SourceLoc>> {
private llvm::TrailingObjects<ImportDecl, Located<Identifier>> {
friend TrailingObjects;
friend class Decl;
public:
typedef std::pair<Identifier, SourceLoc> AccessPathElement;
typedef Located<Identifier> AccessPathElement;

private:
SourceLoc ImportLoc;
Expand Down Expand Up @@ -1577,9 +1578,9 @@ class ImportDecl final : public Decl,
}

SourceLoc getStartLoc() const { return ImportLoc; }
SourceLoc getLocFromSource() const { return getFullAccessPath().front().second; }
SourceLoc getLocFromSource() const { return getFullAccessPath().front().Loc; }
SourceRange getSourceRange() const {
return SourceRange(ImportLoc, getFullAccessPath().back().second);
return SourceRange(ImportLoc, getFullAccessPath().back().Loc);
}
SourceLoc getKindLoc() const { return KindLoc; }

Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ enum class ResilienceStrategy : unsigned {
/// \sa FileUnit
class ModuleDecl : public DeclContext, public TypeDecl {
public:
typedef ArrayRef<std::pair<Identifier, SourceLoc>> AccessPathTy;
typedef ArrayRef<Located<Identifier>> AccessPathTy;
typedef std::pair<ModuleDecl::AccessPathTy, ModuleDecl*> ImportedModule;

static bool matchesAccessPath(AccessPathTy AccessPath, DeclName Name) {
assert(AccessPath.size() <= 1 && "can only refer to top-level decls");

return AccessPath.empty()
|| DeclName(AccessPath.front().first).matchesRef(Name);
|| DeclName(AccessPath.front().Item).matchesRef(Name);
}

/// Arbitrarily orders ImportedModule records, for inclusion in sets and such.
Expand Down
5 changes: 3 additions & 2 deletions include/swift/AST/ModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "swift/AST/Identifier.h"
#include "swift/Basic/LLVM.h"
#include "swift/Basic/Located.h"
#include "swift/Basic/SourceLoc.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallSet.h"
Expand Down Expand Up @@ -100,7 +101,7 @@ class ModuleLoader {
///
/// Note that even if this check succeeds, errors may still occur if the
/// module is loaded in full.
virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) = 0;
virtual bool canImportModule(Located<Identifier> named) = 0;

/// Import a module with the given module path.
///
Expand All @@ -113,7 +114,7 @@ class ModuleLoader {
/// emits a diagnostic and returns NULL.
virtual
ModuleDecl *loadModule(SourceLoc importLoc,
ArrayRef<std::pair<Identifier, SourceLoc>> path) = 0;
ArrayRef<Located<Identifier>> path) = 0;

/// Load extensions to the given nominal type.
///
Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/NameLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -495,15 +495,15 @@ void recordLookupOfTopLevelName(DeclContext *topLevelContext, DeclName name,
void getDirectlyInheritedNominalTypeDecls(
llvm::PointerUnion<TypeDecl *, ExtensionDecl *> decl,
unsigned i,
llvm::SmallVectorImpl<std::pair<SourceLoc, NominalTypeDecl *>> &result,
llvm::SmallVectorImpl<Located<NominalTypeDecl *>> &result,
bool &anyObject);

/// Retrieve the set of nominal type declarations that are directly
/// "inherited" by the given declaration, looking through typealiases
/// and splitting out the components of compositions.
///
/// If we come across the AnyObject type, set \c anyObject true.
SmallVector<std::pair<SourceLoc, NominalTypeDecl *>, 4>
SmallVector<Located<NominalTypeDecl *>, 4>
getDirectlyInheritedNominalTypeDecls(
llvm::PointerUnion<TypeDecl *, ExtensionDecl *> decl,
bool &anyObject);
Expand Down
13 changes: 7 additions & 6 deletions include/swift/AST/TypeRepr.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "llvm/ADT/PointerUnion.h"
#include "llvm/ADT/STLExtras.h"
#include "swift/Basic/Debug.h"
#include "swift/Basic/Located.h"
#include "swift/Basic/InlineBitfield.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TrailingObjects.h"
Expand Down Expand Up @@ -672,9 +673,9 @@ struct TupleTypeReprElement {
/// \endcode
class TupleTypeRepr final : public TypeRepr,
private llvm::TrailingObjects<TupleTypeRepr, TupleTypeReprElement,
std::pair<SourceLoc, unsigned>> {
Located<unsigned>> {
friend TrailingObjects;
typedef std::pair<SourceLoc, unsigned> SourceLocAndIdx;
typedef Located<unsigned> SourceLocAndIdx;

SourceRange Parens;

Expand Down Expand Up @@ -745,21 +746,21 @@ class TupleTypeRepr final : public TypeRepr,

SourceLoc getEllipsisLoc() const {
return hasEllipsis() ?
getTrailingObjects<SourceLocAndIdx>()[0].first : SourceLoc();
getTrailingObjects<SourceLocAndIdx>()[0].Loc : SourceLoc();
}

unsigned getEllipsisIndex() const {
return hasEllipsis() ?
getTrailingObjects<SourceLocAndIdx>()[0].second :
getTrailingObjects<SourceLocAndIdx>()[0].Item :
Bits.TupleTypeRepr.NumElements;
}

void removeEllipsis() {
if (hasEllipsis()) {
Bits.TupleTypeRepr.HasEllipsis = false;
getTrailingObjects<SourceLocAndIdx>()[0] = {
SourceLoc(),
getNumElements()
getNumElements(),
SourceLoc()
};
}
}
Expand Down
86 changes: 86 additions & 0 deletions include/swift/Basic/Located.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
//===--- Located.h - Source Location and Associated Value ----------*- C++ -*-===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2019 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//
//
// Provides a currency data type Located<T> that should be used instead
// of std::pair<T, SourceLoc>.
//
//===----------------------------------------------------------------------===//


#ifndef SWIFT_BASIC_LOCATED_H
#define SWIFT_BASIC_LOCATED_H
#include "swift/Basic/Debug.h"
#include "swift/Basic/LLVM.h"
#include "swift/Basic/SourceLoc.h"

namespace swift {

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

/// The main item whose source location is being tracked.
T Item;

/// The original source location from which the item was parsed.
SourceLoc Loc;

Located() : Item(), Loc() {}

Located(T Item, SourceLoc loc) : Item(Item), Loc(loc) {}

SWIFT_DEBUG_DUMP;
void dump(raw_ostream &os) const;

template<typename U>
friend bool operator ==(const Located<U> &lhs, const Located<U> &rhs) {
return lhs.Item == rhs.Item && lhs.Loc == rhs.Loc;
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a dump method that does something reasonable like for other types? You could have two overloads: void dump() const; void dump(llvm::raw_ostream &OS) const where the former calls the latter using llvm::errs(). [For example, SourceFile does this.]

You could add a new file Located.cpp which has the implementations and the appropriate #includes. You will also need to add Located.cpp to the lib/Basic/CMakeLists.txt file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kitaisreal what do you think about this point? I think it might be useful to have this method for debugging.

As to what is should print, perhaps something that prints the SourceLoc first and then the item, leaving a space in between (say)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@varungandhi-apple I agree, that we should introduce such function. Added.


} // end namespace swift

namespace llvm {

template <typename T> struct DenseMapInfo;

template<typename T>
struct DenseMapInfo<swift::Located<T>> {

static inline swift::Located<T> getEmptyKey() {
return swift::Located<T>(DenseMapInfo<T>::getEmptyKey(),
DenseMapInfo<swift::SourceLoc>::getEmptyKey());
}

static inline swift::Located<T> getTombstoneKey() {
return swift::Located<T>(DenseMapInfo<T>::getTombstoneKey(),
DenseMapInfo<swift::SourceLoc>::getTombstoneKey());
}

static unsigned getHashValue(const swift::Located<T> &LocatedVal) {
return combineHashValue(DenseMapInfo<T>::getHashValue(LocatedVal.Item),
DenseMapInfo<swift::SourceLoc>::getHashValue(LocatedVal.Loc));
}

static bool isEqual(const swift::Located<T> &LHS, const swift::Located<T> &RHS) {
return DenseMapInfo<T>::isEqual(LHS.Item, RHS.Item) &&
DenseMapInfo<T>::isEqual(LHS.Loc, RHS.Loc);
}
};
} // namespace llvm

#endif // SWIFT_BASIC_LOCATED_H
6 changes: 3 additions & 3 deletions include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class ClangImporter final : public ClangModuleLoader {
///
/// Note that even if this check succeeds, errors may still occur if the
/// module is loaded in full.
virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) override;
virtual bool canImportModule(Located<Identifier> named) override;

/// Import a module with the given module path.
///
Expand All @@ -189,7 +189,7 @@ class ClangImporter final : public ClangModuleLoader {
/// emits a diagnostic and returns NULL.
virtual ModuleDecl *loadModule(
SourceLoc importLoc,
ArrayRef<std::pair<Identifier, SourceLoc>> path)
ArrayRef<Located<Identifier>> path)
override;

/// Determine whether \c overlayDC is within an overlay module for the
Expand Down Expand Up @@ -399,7 +399,7 @@ class ClangImporter final : public ClangModuleLoader {
/// Given the path of a Clang module, collect the names of all its submodules.
/// Calling this function does not load the module.
void collectSubModuleNames(
ArrayRef<std::pair<Identifier, SourceLoc>> path,
ArrayRef<Located<Identifier>> path,
std::vector<std::string> &names) const;

/// Given a Clang module, decide whether this module is imported already.
Expand Down
2 changes: 1 addition & 1 deletion include/swift/IDE/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ class NameMatcher: public ASTWalker {

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

Expand Down
2 changes: 1 addition & 1 deletion include/swift/Parse/CodeCompletionCallbacks.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class CodeCompletionCallbacks {

/// Complete the import decl with importable modules.
virtual void
completeImportDecl(std::vector<std::pair<Identifier, SourceLoc>> &Path) {};
completeImportDecl(std::vector<Located<Identifier>> &Path) {};

/// Complete unresolved members after dot.
virtual void completeUnresolvedMember(CodeCompletionExpr *E,
Expand Down
2 changes: 1 addition & 1 deletion include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class Parser {
DeclContext *CurDeclContext;
ASTContext &Context;
CodeCompletionCallbacks *CodeCompletion = nullptr;
std::vector<std::pair<SourceLoc, std::vector<ParamDecl*>>> AnonClosureVars;
std::vector<Located<std::vector<ParamDecl*>>> AnonClosureVars;

bool IsParsingInterfaceTokens = false;

Expand Down
4 changes: 2 additions & 2 deletions include/swift/Sema/SourceLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class SourceLoader : public ModuleLoader {
///
/// Note that even if this check succeeds, errors may still occur if the
/// module is loaded in full.
virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) override;
virtual bool canImportModule(Located<Identifier> named) override;

/// Import a module with the given module path.
///
Expand All @@ -69,7 +69,7 @@ class SourceLoader : public ModuleLoader {
/// returns NULL.
virtual ModuleDecl *
loadModule(SourceLoc importLoc,
ArrayRef<std::pair<Identifier, SourceLoc>> path) override;
ArrayRef<Located<Identifier>> path) override;

/// Load extensions to the given nominal type.
///
Expand Down
10 changes: 5 additions & 5 deletions include/swift/Serialization/SerializedModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
void collectVisibleTopLevelModuleNamesImpl(SmallVectorImpl<Identifier> &names,
StringRef extension) const;

using AccessPathElem = std::pair<Identifier, SourceLoc>;
using AccessPathElem = Located<Identifier>;
bool findModule(AccessPathElem moduleID,
SmallVectorImpl<char> *moduleInterfacePath,
std::unique_ptr<llvm::MemoryBuffer> *moduleBuffer,
Expand Down Expand Up @@ -140,7 +140,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
///
/// Note that even if this check succeeds, errors may still occur if the
/// module is loaded in full.
virtual bool canImportModule(std::pair<Identifier, SourceLoc> named) override;
virtual bool canImportModule(Located<Identifier> named) override;

/// Import a module with the given module path.
///
Expand All @@ -153,7 +153,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
/// emits a diagnostic and returns a FailedImportModule object.
virtual ModuleDecl *
loadModule(SourceLoc importLoc,
ArrayRef<std::pair<Identifier, SourceLoc>> path) override;
ArrayRef<Located<Identifier>> path) override;


virtual void loadExtensions(NominalTypeDecl *nominal,
Expand Down Expand Up @@ -240,10 +240,10 @@ class MemoryBufferSerializedModuleLoader : public SerializedModuleLoaderBase {
public:
virtual ~MemoryBufferSerializedModuleLoader();

bool canImportModule(std::pair<Identifier, SourceLoc> named) override;
bool canImportModule(Located<Identifier> named) override;
ModuleDecl *
loadModule(SourceLoc importLoc,
ArrayRef<std::pair<Identifier, SourceLoc>> path) override;
ArrayRef<Located<Identifier>> path) override;

/// Register a memory buffer that contains the serialized module for the given
/// access path. This API is intended to be used by LLDB to add swiftmodules
Expand Down
Loading