Skip to content

A couple of cleanups around operator and precedence groups #31467

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 3 commits into from
May 1, 2020
Merged

A couple of cleanups around operator and precedence groups #31467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6205aac
[Sema] Factor out lookupPrecedenceGroupForRelation
hamishknight May 1, 2020
b7320cd
[AST] Collapse two arrays into one
hamishknight May 1, 2020
382fcd7
[Sema] Remove some dead code
hamishknight May 1, 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
35 changes: 13 additions & 22 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7156,18 +7156,16 @@ class OperatorDecl : public Decl {

Identifier name;

ArrayRef<Identifier> Identifiers;
ArrayRef<SourceLoc> IdentifierLocs;
ArrayRef<Located<Identifier>> Identifiers;
ArrayRef<NominalTypeDecl *> DesignatedNominalTypes;
SourceLoc getLocFromSource() const { return NameLoc; }
friend class Decl;
public:
OperatorDecl(DeclKind kind, DeclContext *DC, SourceLoc OperatorLoc,
Identifier Name, SourceLoc NameLoc,
ArrayRef<Identifier> Identifiers,
ArrayRef<SourceLoc> IdentifierLocs)
ArrayRef<Located<Identifier>> Identifiers)
: Decl(kind, DC), OperatorLoc(OperatorLoc), NameLoc(NameLoc), name(Name),
Identifiers(Identifiers), IdentifierLocs(IdentifierLocs) {}
Identifiers(Identifiers) {}

OperatorDecl(DeclKind kind, DeclContext *DC, SourceLoc OperatorLoc,
Identifier Name, SourceLoc NameLoc,
Expand Down Expand Up @@ -7202,14 +7200,10 @@ class OperatorDecl : public Decl {
///
/// \todo These two purposes really ought to be in separate properties and the
/// designated type list should be of TypeReprs instead of Identifiers.
ArrayRef<Identifier> getIdentifiers() const {
ArrayRef<Located<Identifier>> getIdentifiers() const {
return Identifiers;
}

ArrayRef<SourceLoc> getIdentifierLocs() const {
return IdentifierLocs;
}

ArrayRef<NominalTypeDecl *> getDesignatedNominalTypes() const {
return DesignatedNominalTypes;
}
Expand Down Expand Up @@ -7238,18 +7232,17 @@ class InfixOperatorDecl : public OperatorDecl {
public:
InfixOperatorDecl(DeclContext *DC, SourceLoc operatorLoc, Identifier name,
SourceLoc nameLoc, SourceLoc colonLoc,
ArrayRef<Identifier> identifiers,
ArrayRef<SourceLoc> identifierLocs)
ArrayRef<Located<Identifier>> identifiers)
: OperatorDecl(DeclKind::InfixOperator, DC, operatorLoc, name, nameLoc,
identifiers, identifierLocs),
identifiers),
ColonLoc(colonLoc) {}

SourceLoc getEndLoc() const {
auto identifierLocs = getIdentifierLocs();
if (identifierLocs.empty())
auto identifiers = getIdentifiers();
if (identifiers.empty())
return getNameLoc();

return identifierLocs.back();
return identifiers.back().Loc;
}

SourceRange getSourceRange() const {
Expand Down Expand Up @@ -7280,10 +7273,9 @@ class PrefixOperatorDecl : public OperatorDecl {
public:
PrefixOperatorDecl(DeclContext *DC, SourceLoc OperatorLoc, Identifier Name,
SourceLoc NameLoc,
ArrayRef<Identifier> Identifiers,
ArrayRef<SourceLoc> IdentifierLocs)
ArrayRef<Located<Identifier>> Identifiers)
: OperatorDecl(DeclKind::PrefixOperator, DC, OperatorLoc, Name, NameLoc,
Identifiers, IdentifierLocs) {}
Identifiers) {}

PrefixOperatorDecl(DeclContext *DC, SourceLoc OperatorLoc, Identifier Name,
SourceLoc NameLoc,
Expand Down Expand Up @@ -7315,10 +7307,9 @@ class PostfixOperatorDecl : public OperatorDecl {
public:
PostfixOperatorDecl(DeclContext *DC, SourceLoc OperatorLoc, Identifier Name,
SourceLoc NameLoc,
ArrayRef<Identifier> Identifiers,
ArrayRef<SourceLoc> IdentifierLocs)
ArrayRef<Located<Identifier>> Identifiers)
: OperatorDecl(DeclKind::PostfixOperator, DC, OperatorLoc, Name, NameLoc,
Identifiers, IdentifierLocs) {}
Identifiers) {}

PostfixOperatorDecl(DeclContext *DC, SourceLoc OperatorLoc, Identifier Name,
SourceLoc NameLoc,
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTDumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ namespace {
auto identifiers = OD->getIdentifiers();
for (auto index : indices(identifiers)) {
OS.indent(Indent + 2);
OS << "identifier #" << index << " " << identifiers[index];
OS << "identifier #" << index << " " << identifiers[index].Item;
if (index != identifiers.size() - 1)
OS << "\n";
}
Expand Down
29 changes: 13 additions & 16 deletions lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7530,8 +7530,7 @@ Parser::parseDeclOperatorImpl(SourceLoc OperatorLoc, Identifier Name,
// parse them both as identifiers here and sort it out in type
// checking.
SourceLoc colonLoc;
SmallVector<Identifier, 4> identifiers;
SmallVector<SourceLoc, 4> identifierLocs;
SmallVector<Located<Identifier>, 4> identifiers;
if (Tok.is(tok::colon)) {
SyntaxParsingContext GroupCtxt(SyntaxContext,
SyntaxKind::OperatorPrecedenceAndTypes);
Expand All @@ -7552,16 +7551,16 @@ Parser::parseDeclOperatorImpl(SourceLoc OperatorLoc, Identifier Name,
SyntaxKind::IdentifierList);

Identifier name;
identifierLocs.push_back(consumeIdentifier(&name));
identifiers.push_back(name);
auto loc = consumeIdentifier(&name);
identifiers.emplace_back(name, loc);

while (Tok.is(tok::comma)) {
auto comma = consumeToken();

if (Tok.is(tok::identifier)) {
Identifier name;
identifierLocs.push_back(consumeIdentifier(&name));
identifiers.push_back(name);
auto loc = consumeIdentifier(&name);
identifiers.emplace_back(name, loc);
} else {
if (Tok.isNot(tok::eof)) {
auto otherTokLoc = consumeToken();
Expand All @@ -7576,12 +7575,13 @@ Parser::parseDeclOperatorImpl(SourceLoc OperatorLoc, Identifier Name,
SyntaxParsingContext GroupCtxt(SyntaxContext,
SyntaxKind::IdentifierList);

identifiers.push_back(Context.getIdentifier(Tok.getText()));
identifierLocs.push_back(consumeToken(tok::identifier));
Identifier name;
auto nameLoc = consumeIdentifier(&name);
identifiers.emplace_back(name, nameLoc);

if (isPrefix || isPostfix) {
diagnose(colonLoc, diag::precedencegroup_not_infix)
.fixItRemove({colonLoc, identifierLocs.back()});
.fixItRemove({colonLoc, nameLoc});
}
// Nothing to complete here, simply consume the token.
if (Tok.is(tok::code_complete))
Expand All @@ -7598,7 +7598,7 @@ Parser::parseDeclOperatorImpl(SourceLoc OperatorLoc, Identifier Name,
auto Diag = diagnose(lBraceLoc, diag::deprecated_operator_body);
if (Tok.is(tok::r_brace)) {
SourceLoc lastGoodLoc =
!identifierLocs.empty() ? identifierLocs.back() : SourceLoc();
!identifiers.empty() ? identifiers.back().Loc : SourceLoc();
if (lastGoodLoc.isInvalid())
lastGoodLoc = NameLoc;
SourceLoc lastGoodLocEnd = Lexer::getLocForEndOfToken(SourceMgr,
Expand All @@ -7616,18 +7616,15 @@ Parser::parseDeclOperatorImpl(SourceLoc OperatorLoc, Identifier Name,
if (Attributes.hasAttribute<PrefixAttr>())
res = new (Context)
PrefixOperatorDecl(CurDeclContext, OperatorLoc, Name, NameLoc,
Context.AllocateCopy(identifiers),
Context.AllocateCopy(identifierLocs));
Context.AllocateCopy(identifiers));
else if (Attributes.hasAttribute<PostfixAttr>())
res = new (Context)
PostfixOperatorDecl(CurDeclContext, OperatorLoc, Name, NameLoc,
Context.AllocateCopy(identifiers),
Context.AllocateCopy(identifierLocs));
Context.AllocateCopy(identifiers));
else
res = new (Context)
InfixOperatorDecl(CurDeclContext, OperatorLoc, Name, NameLoc, colonLoc,
Context.AllocateCopy(identifiers),
Context.AllocateCopy(identifierLocs));
Context.AllocateCopy(identifiers));

diagnoseOperatorFixityAttributes(*this, Attributes, res);

Expand Down
4 changes: 2 additions & 2 deletions lib/Sema/TypeCheckAccess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2006,9 +2006,9 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
// FIXME: Handle operator designated types (which also applies to prefix
// and postfix operators).
if (auto *precedenceGroup = IOD->getPrecedenceGroup()) {
if (!IOD->getIdentifierLocs().empty()) {
if (!IOD->getIdentifiers().empty()) {
checkPrecedenceGroup(precedenceGroup, IOD, IOD->getLoc(),
IOD->getIdentifierLocs().front());
IOD->getIdentifiers().front().Loc);
}
}
}
Expand Down
127 changes: 61 additions & 66 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1336,30 +1336,46 @@ lookupPrecedenceGroup(const PrecedenceGroupDescriptor &descriptor) {
}
}

static PrecedenceGroupDecl *lookupPrecedenceGroupForRelation(
DeclContext *dc, PrecedenceGroupDecl::Relation rel,
PrecedenceGroupDescriptor::PathDirection direction) {
auto &ctx = dc->getASTContext();
PrecedenceGroupDescriptor desc{dc, rel.Name, rel.NameLoc, direction};
auto result = ctx.evaluator(ValidatePrecedenceGroupRequest{desc});
if (!result) {
// Handle a cycle error specially. We don't want to default to an empty
// result, as we don't want to emit an error about not finding a precedence
// group.
using Error = CyclicalRequestError<ValidatePrecedenceGroupRequest>;
llvm::handleAllErrors(result.takeError(), [](const Error &E) {});
return nullptr;
}
if (!result.get()) {
ctx.Diags.diagnose(rel.NameLoc, diag::unknown_precedence_group, rel.Name);
}
return result.get();
}

void swift::validatePrecedenceGroup(PrecedenceGroupDecl *PGD) {
assert(PGD && "Cannot validate a null precedence group!");
if (PGD->isInvalid())
return;

auto &Diags = PGD->getASTContext().Diags;
auto *dc = PGD->getDeclContext();

// Validate the higherThan relationships.
bool addedHigherThan = false;
for (auto &rel : PGD->getMutableHigherThan()) {
if (rel.Group)
continue;

PrecedenceGroupDescriptor desc{PGD->getDeclContext(), rel.Name, rel.NameLoc,
PrecedenceGroupDescriptor::HigherThan};
auto group =
evaluateOrDefault(PGD->getASTContext().evaluator,
ValidatePrecedenceGroupRequest{desc}, nullptr);
if (group) {
rel.Group = group;
// TODO: Requestify the lookup of a relation's group.
rel.Group = lookupPrecedenceGroupForRelation(
dc, rel, PrecedenceGroupDescriptor::HigherThan);
if (rel.Group) {
addedHigherThan = true;
} else {
if (!lookupPrecedenceGroup(desc))
Diags.diagnose(rel.NameLoc, diag::unknown_precedence_group, rel.Name);
PGD->setInvalid();
}
}
Expand All @@ -1369,24 +1385,16 @@ void swift::validatePrecedenceGroup(PrecedenceGroupDecl *PGD) {
if (rel.Group)
continue;

auto dc = PGD->getDeclContext();
PrecedenceGroupDescriptor desc{PGD->getDeclContext(), rel.Name, rel.NameLoc,
PrecedenceGroupDescriptor::LowerThan};
auto group =
evaluateOrDefault(PGD->getASTContext().evaluator,
ValidatePrecedenceGroupRequest{desc}, nullptr);
bool hadError = false;
if (group) {
rel.Group = group;
} else {
hadError = true;
if (auto *rawGroup = lookupPrecedenceGroup(desc)) {
// We already know the lowerThan path is errant, try to use the results
// of a raw lookup to enforce the same-module restriction.
group = rawGroup;
} else {
Diags.diagnose(rel.NameLoc, diag::unknown_precedence_group, rel.Name);
}
auto *group = lookupPrecedenceGroupForRelation(
dc, rel, PrecedenceGroupDescriptor::LowerThan);
rel.Group = group;

// If we didn't find anything, try doing a raw lookup for the group before
// diagnosing the 'lowerThan' within the same-module restriction. This can
// allow us to diagnose even if we have a precedence group cycle.
if (!group) {
group = lookupPrecedenceGroup(PrecedenceGroupDescriptor{
dc, rel.Name, rel.NameLoc, PrecedenceGroupDescriptor::LowerThan});
}

if (group &&
Expand All @@ -1396,10 +1404,10 @@ void swift::validatePrecedenceGroup(PrecedenceGroupDecl *PGD) {
Diags.diagnose(group->getNameLoc(), diag::kind_declared_here,
DescriptiveDeclKind::PrecedenceGroup);
}
hadError = true;
PGD->setInvalid();
}

if (hadError)
if (!rel.Group)
PGD->setInvalid();
}

Expand Down Expand Up @@ -1441,18 +1449,13 @@ static NominalTypeDecl *resolveSingleNominalTypeDecl(
}

bool swift::checkDesignatedTypes(OperatorDecl *OD,
ArrayRef<Identifier> identifiers,
ArrayRef<SourceLoc> identifierLocs,
ASTContext &ctx) {
assert(identifiers.size() == identifierLocs.size());

SmallVector<NominalTypeDecl *, 1> designatedNominalTypes;
ArrayRef<Located<Identifier>> identifiers) {
auto &ctx = OD->getASTContext();
auto *DC = OD->getDeclContext();

for (auto index : indices(identifiers)) {
auto *decl = resolveSingleNominalTypeDecl(DC, identifierLocs[index],
identifiers[index], ctx);

SmallVector<NominalTypeDecl *, 1> designatedNominalTypes;
for (auto ident : identifiers) {
auto *decl = resolveSingleNominalTypeDecl(DC, ident.Loc, ident.Item, ctx);
if (!decl)
return true;

Expand All @@ -1471,63 +1474,55 @@ bool swift::checkDesignatedTypes(OperatorDecl *OD,
PrecedenceGroupDecl *
OperatorPrecedenceGroupRequest::evaluate(Evaluator &evaluator,
InfixOperatorDecl *IOD) const {
auto &ctx = IOD->getASTContext();
auto *dc = IOD->getDeclContext();

auto enableOperatorDesignatedTypes =
IOD->getASTContext().TypeCheckerOpts.EnableOperatorDesignatedTypes;
ctx.TypeCheckerOpts.EnableOperatorDesignatedTypes;

auto &Diags = IOD->getASTContext().Diags;
auto &Diags = ctx.Diags;
PrecedenceGroupDecl *group = nullptr;

auto identifiers = IOD->getIdentifiers();
auto identifierLocs = IOD->getIdentifierLocs();

if (!identifiers.empty()) {
group = TypeChecker::lookupPrecedenceGroup(
IOD->getDeclContext(), identifiers[0], identifierLocs[0]);
auto name = identifiers[0].Item;
auto loc = identifiers[0].Loc;

group = TypeChecker::lookupPrecedenceGroup(dc, name, loc);

if (group) {
identifiers = identifiers.slice(1);
identifierLocs = identifierLocs.slice(1);
} else {
// If we're either not allowing types, or we are allowing them
// and this identifier is not a type, emit an error as if it's
// a precedence group.
auto *DC = IOD->getDeclContext();
if (!(enableOperatorDesignatedTypes &&
resolveSingleNominalTypeDecl(DC, identifierLocs[0], identifiers[0],
IOD->getASTContext(),
resolveSingleNominalTypeDecl(dc, loc, name, ctx,
TypeResolutionFlags::SilenceErrors))) {
Diags.diagnose(identifierLocs[0], diag::unknown_precedence_group,
identifiers[0]);
Diags.diagnose(loc, diag::unknown_precedence_group, name);
identifiers = identifiers.slice(1);
identifierLocs = identifierLocs.slice(1);
}
}
}

if (!identifiers.empty() && !enableOperatorDesignatedTypes) {
assert(!group);
Diags.diagnose(identifierLocs[0], diag::unknown_precedence_group,
identifiers[0]);
identifiers = identifiers.slice(1);
identifierLocs = identifierLocs.slice(1);
assert(identifiers.empty() && identifierLocs.empty());
}
// Unless operator designed types are enabled, the parser will ensure that
// only one identifier is allowed in the clause, which we should have just
// handled.
assert(identifiers.empty() || enableOperatorDesignatedTypes);

if (!group) {
group = TypeChecker::lookupPrecedenceGroup(
IOD->getDeclContext(), IOD->getASTContext().Id_DefaultPrecedence,
SourceLoc());
group = TypeChecker::lookupPrecedenceGroup(dc, ctx.Id_DefaultPrecedence,
SourceLoc());
}

if (!group) {
Diags.diagnose(IOD->getLoc(), diag::missing_builtin_precedence_group,
IOD->getASTContext().Id_DefaultPrecedence);
ctx.Id_DefaultPrecedence);
}

auto nominalTypes = IOD->getDesignatedNominalTypes();
if (nominalTypes.empty() && enableOperatorDesignatedTypes) {
if (checkDesignatedTypes(IOD, identifiers, identifierLocs,
IOD->getASTContext())) {
if (checkDesignatedTypes(IOD, identifiers)) {
IOD->setInvalid();
}
}
Expand Down
Loading