Skip to content

[NFC] Remove SourceFile's Operator Lookup Entrypoints #30547

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.

Sign up for GitHub

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
merged 2 commits into from
Mar 21, 2020
Merged

[NFC] Remove SourceFile's Operator Lookup Entrypoints #30547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c136a0b
[NFC] Remove SourceFile's Operator Lookup Entrypoints
CodaFi Mar 21, 2020
425bd50
[Gardening] Remove some parameters obviated by OperatorLookup::map_ptr
CodaFi Mar 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions include/swift/AST/SourceFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,25 +461,6 @@ class SourceFile final : public FileUnit {

virtual bool walk(ASTWalker &walker) override;

/// @{

/// Look up the given operator in this file.
///
/// The file must be name-bound already. If the operator is not found, or if
/// there is an ambiguity, returns null.
///
/// \param isCascading If true, the lookup of this operator may affect
/// downstream files.
InfixOperatorDecl *lookupInfixOperator(Identifier name, bool isCascading,
SourceLoc diagLoc = {});
PrefixOperatorDecl *lookupPrefixOperator(Identifier name, bool isCascading,
SourceLoc diagLoc = {});
PostfixOperatorDecl *lookupPostfixOperator(Identifier name, bool isCascading,
SourceLoc diagLoc = {});
PrecedenceGroupDecl *lookupPrecedenceGroup(Identifier name, bool isCascading,
SourceLoc diagLoc = {});
/// @}

ReferencedNameTracker *getReferencedNameTracker() {
return ReferencedNames ? ReferencedNames.getPointer() : nullptr;
}
Expand Down
35 changes: 13 additions & 22 deletions lib/AST/Module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,8 +1014,7 @@ void SourceFile::setSyntaxRoot(syntax::SourceFileSyntax &&Root) {

template<typename OP_DECL>
static Optional<OP_DECL *>
lookupOperatorDeclForName(ModuleDecl *M, SourceLoc Loc, Identifier Name,
OperatorMap<OP_DECL *> SourceFile::*OP_MAP);
lookupOperatorDeclForName(ModuleDecl *M, SourceLoc Loc, Identifier Name);

template<typename OP_DECL>
using ImportedOperatorsMap = llvm::SmallDenseMap<OP_DECL*, bool, 16>;
Expand Down Expand Up @@ -1066,8 +1065,7 @@ checkOperatorConflicts(const SourceFile &SF, SourceLoc loc,
template <typename OP_DECL>
static Optional<OP_DECL *>
lookupOperatorDeclForName(const FileUnit &File, SourceLoc Loc,
Identifier Name, bool includePrivate,
OperatorMap<OP_DECL *> SourceFile::*OP_MAP) {
Identifier Name, bool includePrivate) {
switch (File.getKind()) {
case FileUnitKind::Builtin:
// The Builtin module declares no operators.
Expand All @@ -1084,6 +1082,7 @@ lookupOperatorDeclForName(const FileUnit &File, SourceLoc Loc,
assert(SF.ASTStage >= SourceFile::NameBound);

// Look for an operator declaration in the current module.
const auto OP_MAP = OperatorLookup<OP_DECL>::map_ptr;
auto found = (SF.*OP_MAP).find(Name);
if (found != (SF.*OP_MAP).end() && (includePrivate || found->second.getInt()))
return found->second.getPointer();
Expand All @@ -1105,7 +1104,7 @@ lookupOperatorDeclForName(const FileUnit &File, SourceLoc Loc,
continue;

Optional<OP_DECL *> maybeOp =
lookupOperatorDeclForName(imported.module.second, Loc, Name, OP_MAP);
lookupOperatorDeclForName<OP_DECL>(imported.module.second, Loc, Name);
if (!maybeOp)
return None;

Expand Down Expand Up @@ -1138,12 +1137,10 @@ lookupOperatorDeclForName(const FileUnit &File, SourceLoc Loc,

template<typename OP_DECL>
static Optional<OP_DECL *>
lookupOperatorDeclForName(ModuleDecl *M, SourceLoc Loc, Identifier Name,
OperatorMap<OP_DECL *> SourceFile::*OP_MAP)
{
lookupOperatorDeclForName(ModuleDecl *M, SourceLoc Loc, Identifier Name) {
OP_DECL *result = nullptr;
for (const FileUnit *File : M->getFiles()) {
auto next = lookupOperatorDeclForName(*File, Loc, Name, false, OP_MAP);
auto next = lookupOperatorDeclForName<OP_DECL>(*File, Loc, Name, false);
if (!next.hasValue())
return next;

Expand All @@ -1159,9 +1156,9 @@ lookupOperatorDeclForName(ModuleDecl *M, SourceLoc Loc, Identifier Name,
template <typename OperatorType>
llvm::Expected<OperatorType *> LookupOperatorRequest<OperatorType>::evaluate(
Evaluator &evaluator, OperatorLookupDescriptor desc) const {
auto result = lookupOperatorDeclForName(*desc.SF, desc.diagLoc, desc.name,
/*includePrivate*/ true,
OperatorLookup<OperatorType>::map_ptr);
auto result = lookupOperatorDeclForName<OperatorType>(*desc.SF, desc.diagLoc,
desc.name,
/*includePrivate*/ true);
if (!result.hasValue())
return nullptr;
if (auto *tracker = desc.SF->getReferencedNameTracker()) {
Expand All @@ -1172,9 +1169,9 @@ llvm::Expected<OperatorType *> LookupOperatorRequest<OperatorType>::evaluate(
}
}
if (!result.getValue()) {
result = lookupOperatorDeclForName(desc.SF->getParentModule(), desc.diagLoc,
desc.name,
OperatorLookup<OperatorType>::map_ptr);
result = lookupOperatorDeclForName<OperatorType>(desc.SF->getParentModule(),
desc.diagLoc,
desc.name);
}
return result.hasValue() ? result.getValue() : nullptr;
}
Expand All @@ -1183,15 +1180,9 @@ llvm::Expected<OperatorType *> LookupOperatorRequest<OperatorType>::evaluate(
#define LOOKUP_OPERATOR(Kind) \
Kind##Decl *ModuleDecl::lookup##Kind(Identifier name, SourceLoc loc) { \
auto result = \
lookupOperatorDeclForName(this, loc, name, &SourceFile::Kind##s); \
lookupOperatorDeclForName<Kind##Decl>(this, loc, name); \
return result ? *result : nullptr; \
} \
Kind##Decl *SourceFile::lookup##Kind(Identifier name, bool cascades, \
SourceLoc loc) { \
return evaluateOrDefault( \
getASTContext().evaluator, \
Lookup##Kind##Request{{this, name, cascades, loc}}, nullptr); \
} \
template llvm::Expected<Kind##Decl *> \
LookupOperatorRequest<Kind##Decl>::evaluate(Evaluator &e, \
OperatorLookupDescriptor d) const;
Expand Down
48 changes: 27 additions & 21 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1253,9 +1253,14 @@ static PrecedenceGroupDecl *
lookupPrecedenceGroup(const PrecedenceGroupDescriptor &descriptor) {
auto *dc = descriptor.dc;
if (auto sf = dc->getParentSourceFile()) {
bool cascading = dc->isCascadingContextForLookup(false);
return sf->lookupPrecedenceGroup(descriptor.ident, cascading,
descriptor.nameLoc);
OperatorLookupDescriptor desc{
sf,
descriptor.ident,
dc->isCascadingContextForLookup(false),
descriptor.nameLoc
};
return evaluateOrDefault(sf->getASTContext().evaluator,
LookupPrecedenceGroupRequest{desc}, nullptr);
} else {
return dc->getParentModule()->lookupPrecedenceGroup(descriptor.ident,
descriptor.nameLoc);
Expand Down Expand Up @@ -1725,26 +1730,27 @@ FunctionOperatorRequest::evaluate(Evaluator &evaluator, FuncDecl *FD) const {
FD->diagnose(diag::operator_in_local_scope);
}

OperatorLookupDescriptor desc{
FD->getDeclContext()->getParentSourceFile(),
operatorName,
FD->isCascadingContextForLookup(false),
FD->getLoc()
};
OperatorDecl *op = nullptr;
SourceFile &SF = *FD->getDeclContext()->getParentSourceFile();
if (FD->isUnaryOperator()) {
if (FD->getAttrs().hasAttribute<PrefixAttr>()) {
op = SF.lookupPrefixOperator(operatorName,
FD->isCascadingContextForLookup(false),
FD->getLoc());
op = evaluateOrDefault(evaluator,
LookupPrefixOperatorRequest{desc}, nullptr);
} else if (FD->getAttrs().hasAttribute<PostfixAttr>()) {
op = SF.lookupPostfixOperator(operatorName,
FD->isCascadingContextForLookup(false),
FD->getLoc());
op = evaluateOrDefault(evaluator,
LookupPostfixOperatorRequest{desc}, nullptr);
} else {
auto prefixOp =
SF.lookupPrefixOperator(operatorName,
FD->isCascadingContextForLookup(false),
FD->getLoc());
auto postfixOp =
SF.lookupPostfixOperator(operatorName,
FD->isCascadingContextForLookup(false),
FD->getLoc());
auto prefixOp = evaluateOrDefault(evaluator,
LookupPrefixOperatorRequest{desc},
nullptr);
auto postfixOp = evaluateOrDefault(evaluator,
LookupPostfixOperatorRequest{desc},
nullptr);

// If we found both prefix and postfix, or neither prefix nor postfix,
// complain. We can't fix this situation.
Expand Down Expand Up @@ -1790,9 +1796,9 @@ FunctionOperatorRequest::evaluate(Evaluator &evaluator, FuncDecl *FD) const {
static_cast<bool>(postfixOp));
}
} else if (FD->isBinaryOperator()) {
op = SF.lookupInfixOperator(operatorName,
FD->isCascadingContextForLookup(false),
FD->getLoc());
op = evaluateOrDefault(evaluator,
LookupInfixOperatorRequest{desc},
nullptr);
} else {
diags.diagnose(FD, diag::invalid_arg_count_for_operator);
return nullptr;
Expand Down
16 changes: 12 additions & 4 deletions lib/Sema/TypeCheckExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "TypeChecker.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/NameLookupRequests.h"
#include "swift/AST/Decl.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/ParameterList.h"
Expand Down Expand Up @@ -129,12 +130,19 @@ Expr *TypeChecker::substituteInputSugarTypeForResult(ApplyExpr *E) {
static PrecedenceGroupDecl *lookupPrecedenceGroupForOperator(DeclContext *DC,
Identifier name,
SourceLoc loc) {
SourceFile *SF = DC->getParentSourceFile();
bool isCascading = DC->isCascadingContextForLookup(true);
if (auto op = SF->lookupInfixOperator(name, isCascading, loc)) {
OperatorLookupDescriptor desc{
DC->getParentSourceFile(),
name,
DC->isCascadingContextForLookup(true),
loc
};
auto &Ctx = DC->getASTContext();
if (auto op = evaluateOrDefault(Ctx.evaluator,
LookupInfixOperatorRequest{desc},
nullptr)) {
return op->getPrecedenceGroup();
} else {
DC->getASTContext().Diags.diagnose(loc, diag::unknown_binop);
Ctx.Diags.diagnose(loc, diag::unknown_binop);
}
return nullptr;
}
Expand Down