Skip to content

Commit c43eb9b

Browse files
authored
Merge pull request #26418 from nathawes/type-attr-coloring
2 parents 13640f6 + 4d2aca1 commit c43eb9b

File tree

10 files changed

+102
-66
lines changed

10 files changed

+102
-66
lines changed

include/swift/AST/Attr.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ class TypeAttributes {
101101
AttrLocs[A] = L;
102102
}
103103

104-
void getAttrRanges(SmallVectorImpl<SourceRange> &Ranges) const {
104+
void getAttrLocs(SmallVectorImpl<SourceLoc> &Locs) const {
105105
for (auto Loc : AttrLocs) {
106106
if (Loc.isValid())
107-
Ranges.push_back(Loc);
107+
Locs.push_back(Loc);
108108
}
109109
}
110110

include/swift/Parse/Parser.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,7 @@ class Parser {
975975
bool parseTypeAttributeListPresent(ParamDecl::Specifier &Specifier,
976976
SourceLoc &SpecifierLoc,
977977
TypeAttributes &Attributes);
978-
bool parseTypeAttribute(TypeAttributes &Attributes,
978+
bool parseTypeAttribute(TypeAttributes &Attributes, SourceLoc AtLoc,
979979
bool justChecking = false);
980980

981981

lib/AST/ASTWalker.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
164164
}
165165

166166
bool visitPatternBindingDecl(PatternBindingDecl *PBD) {
167-
// If there is a single variable, walk it's attributes.
168167
bool isPropertyWrapperBackingProperty = false;
169168
if (auto singleVar = PBD->getSingleVar()) {
170169
isPropertyWrapperBackingProperty =

lib/IDE/SyntaxModel.cpp

Lines changed: 61 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,13 @@ class ModelASTWalker : public ASTWalker {
301301
/// Like ExcludeNodeAtLocation, and skip past any node at the location.
302302
DisplaceNodeAtLocation
303303
};
304-
bool passTokenNodesUntil(SourceLoc Loc, PassNodesBehavior Behavior);
304+
struct PassUntilResult {
305+
bool shouldContinue;
306+
Optional<SyntaxNode> MatchedToken;
307+
};
308+
309+
PassUntilResult
310+
passTokenNodesUntil(SourceLoc Loc, PassNodesBehavior Pass);
305311
bool passNonTokenNode(const SyntaxNode &Node);
306312
bool passNode(const SyntaxNode &Node);
307313
bool pushStructureNode(const SyntaxStructureNode &Node,
@@ -433,8 +439,7 @@ std::pair<bool, Expr *> ModelASTWalker::walkToExprPre(Expr *E) {
433439
if (NR.isValid()) {
434440
SN.Range = charSourceRangeFromSourceRange(SM, SourceRange(NR.getStart(),
435441
Elem->getEndLoc()));
436-
passTokenNodesUntil(NR.getStart(),
437-
PassNodesBehavior::ExcludeNodeAtLocation);
442+
passTokenNodesUntil(NR.getStart(), ExcludeNodeAtLocation);
438443
}
439444
else
440445
SN.Range = SN.BodyRange;
@@ -972,7 +977,8 @@ bool ModelASTWalker::walkToTypeReprPre(TypeRepr *T) {
972977
return false;
973978

974979
} else if (auto IdT = dyn_cast<ComponentIdentTypeRepr>(T)) {
975-
if (!passTokenNodesUntil(IdT->getIdLoc(), ExcludeNodeAtLocation))
980+
if (!passTokenNodesUntil(IdT->getIdLoc(),
981+
ExcludeNodeAtLocation).shouldContinue)
976982
return false;
977983
if (TokenNodes.empty() ||
978984
TokenNodes.front().Range.getStart() != IdT->getIdLoc())
@@ -1025,7 +1031,7 @@ bool ModelASTWalker::handleSpecialDeclAttribute(const DeclAttribute *D,
10251031
return false;
10261032
if (isa<CustomAttr>(D) || isa<AvailableAttr>(D)) {
10271033
if (!passTokenNodesUntil(D->getRangeWithAt().Start,
1028-
PassNodesBehavior::ExcludeNodeAtLocation))
1034+
ExcludeNodeAtLocation).shouldContinue)
10291035
return false;
10301036
if (auto *CA = dyn_cast<CustomAttr>(D)) {
10311037
if (auto *Repr = CA->getTypeLoc().getTypeRepr()) {
@@ -1048,7 +1054,8 @@ bool ModelASTWalker::handleSpecialDeclAttribute(const DeclAttribute *D,
10481054
} else {
10491055
assert(0 && "No TokenNodes?");
10501056
}
1051-
if (!passTokenNodesUntil(D->getRange().End, PassNodesBehavior::IncludeNodeAtLocation))
1057+
if (!passTokenNodesUntil(D->getRange().End,
1058+
IncludeNodeAtLocation).shouldContinue)
10521059
return false;
10531060
return true;
10541061
}
@@ -1067,11 +1074,11 @@ bool ModelASTWalker::handleAttrs(const DeclAttributes &Attrs) {
10671074
}
10681075

10691076
bool ModelASTWalker::handleAttrs(const TypeAttributes &Attrs) {
1070-
SmallVector<SourceRange, 4> Ranges;
1071-
Attrs.getAttrRanges(Ranges);
1077+
SmallVector<SourceLoc, 4> AttrLocs;
1078+
Attrs.getAttrLocs(AttrLocs);
10721079
SmallVector<DeclAttributeAndRange, 4> DeclRanges;
1073-
for (auto R : Ranges) {
1074-
DeclRanges.push_back(std::make_pair(nullptr, R));
1080+
for (auto AttrLoc : AttrLocs) {
1081+
DeclRanges.push_back(std::make_pair(nullptr, SourceRange(AttrLoc)));
10751082
}
10761083
return handleAttrRanges(DeclRanges);
10771084
}
@@ -1099,20 +1106,34 @@ bool ModelASTWalker::handleAttrRanges(ArrayRef<DeclAttributeAndRange> DeclRanges
10991106
DeclRanges = SortedRanges;
11001107

11011108
SourceLoc BeginLoc = DeclRanges.front().second.Start;
1102-
11031109
auto Toks = slice_token_array(AllTokensInFile, BeginLoc,
11041110
DeclRanges.back().second.End);
11051111

11061112
auto passAttrNode = [&](SourceRange AttrRange) -> bool {
11071113
SourceRange Range = AttrRange;
1108-
if (!passNonTokenNode({SyntaxNodeKind::AttributeBuiltin,
1109-
charSourceRangeFromSourceRange(SM, Range)}))
1114+
auto PassUntilResult = passTokenNodesUntil(Range.Start,
1115+
ExcludeNodeAtLocation);
1116+
if (!PassUntilResult.shouldContinue)
11101117
return false;
11111118

1112-
while (!TokenNodes.empty() &&
1113-
SM.rangeContainsTokenLoc(AttrRange,
1114-
TokenNodes.front().Range.getStart()))
1115-
TokenNodes = TokenNodes.slice(1);
1119+
if (PassUntilResult.MatchedToken) {
1120+
// Type attribute ranges don't have the correct end location (it only
1121+
// covers the @ itself), so use matched token's range instead.
1122+
CharSourceRange AdjustedRange = charSourceRangeFromSourceRange(SM, Range);
1123+
AdjustedRange.widen(PassUntilResult.MatchedToken->Range);
1124+
if (!passNode({SyntaxNodeKind::AttributeBuiltin, AdjustedRange}))
1125+
return false;
1126+
TokenNodes = TokenNodes.drop_while([&](SyntaxNode TokenNode) {
1127+
return AdjustedRange.contains(TokenNode.Range.getStart());
1128+
});
1129+
} else {
1130+
// Make sure we're revisiting something, rather than dealing with bad
1131+
// source locations
1132+
assert((TokenNodes.empty() ||
1133+
SM.isBeforeInBuffer(AttrRange.End,
1134+
TokenNodes.front().Range.getStart())) &&
1135+
"AttrRange doesn't align with any TokenNode?");
1136+
}
11161137
return true;
11171138
};
11181139

@@ -1146,31 +1167,36 @@ bool ModelASTWalker::shouldPassBraceStructureNode(BraceStmt *S) {
11461167
S->getSourceRange().isValid());
11471168
}
11481169

1149-
bool ModelASTWalker::passTokenNodesUntil(SourceLoc Loc,
1150-
PassNodesBehavior Behavior) {
1170+
ModelASTWalker::PassUntilResult
1171+
ModelASTWalker::passTokenNodesUntil(SourceLoc Loc,
1172+
PassNodesBehavior Behavior) {
11511173
assert(Loc.isValid());
11521174
unsigned I = 0;
1175+
Optional<SyntaxNode> MatchedToken;
11531176
for (unsigned E = TokenNodes.size(); I != E; ++I) {
1154-
SourceLoc TokLoc = TokenNodes[I].Range.getStart();
1155-
if (SM.isBeforeInBuffer(Loc, TokLoc)) {
1177+
SourceLoc StartLoc = TokenNodes[I].Range.getStart();
1178+
if (SM.isBeforeInBuffer(Loc, StartLoc)) {
11561179
break;
11571180
}
1158-
if (TokLoc == Loc && Behavior != IncludeNodeAtLocation) {
1159-
if (Behavior == DisplaceNodeAtLocation) {
1160-
// Skip past the node directly at the specified location, allowing the
1161-
// caller to effectively replace it.
1162-
++I;
1181+
if (StartLoc == Loc) {
1182+
MatchedToken = TokenNodes[I];
1183+
if (Behavior != IncludeNodeAtLocation) {
1184+
if (Behavior == DisplaceNodeAtLocation) {
1185+
// Skip past the node directly at the specified location, allowing the
1186+
// caller to effectively replace it.
1187+
++I;
1188+
}
1189+
break;
11631190
}
1164-
break;
11651191
}
11661192
if (!AvoidPassingSyntaxToken) {
11671193
if (!passNode(TokenNodes[I]))
1168-
return false;
1194+
return {false, None};
11691195
}
11701196
}
11711197

11721198
TokenNodes = TokenNodes.slice(I);
1173-
return true;
1199+
return {true, MatchedToken};
11741200
}
11751201

11761202
bool ModelASTWalker::passNonTokenNode(const SyntaxNode &Node) {
@@ -1181,7 +1207,8 @@ bool ModelASTWalker::passNonTokenNode(const SyntaxNode &Node) {
11811207
if (!SM.isBeforeInBuffer(LastLoc, Node.Range.getStart()))
11821208
return true;
11831209

1184-
if (!passTokenNodesUntil(Node.Range.getStart(), DisplaceNodeAtLocation))
1210+
if (!passTokenNodesUntil(Node.Range.getStart(),
1211+
DisplaceNodeAtLocation).shouldContinue)
11851212
return false;
11861213
if (!passNode(Node))
11871214
return false;
@@ -1223,7 +1250,8 @@ bool ModelASTWalker::pushStructureNode(const SyntaxStructureNode &Node,
12231250
if (shouldAvoidPssingSyntaxToken(Node))
12241251
AvoidPassingSyntaxToken ++;
12251252

1226-
if (!passTokenNodesUntil(Node.Range.getStart(), ExcludeNodeAtLocation))
1253+
if (!passTokenNodesUntil(Node.Range.getStart(),
1254+
ExcludeNodeAtLocation).shouldContinue)
12271255
return false;
12281256
if (!Walker.walkToSubStructurePre(Node))
12291257
return false;
@@ -1245,7 +1273,8 @@ bool ModelASTWalker::popStructureNode() {
12451273
// VarDecls are popped before we see their TypeRepr, so if we pass the token
12461274
// nodes now they will not change from identifier to a type-identifier.
12471275
if (!Node.hasSubstructure()) {
1248-
if (!passTokenNodesUntil(Node.Range.getEnd(), IncludeNodeAtLocation))
1276+
if (!passTokenNodesUntil(Node.Range.getEnd(),
1277+
IncludeNodeAtLocation).shouldContinue)
12491278
return false;
12501279
}
12511280
if (!Walker.walkToSubStructurePost(Node))

lib/Parse/ParseDecl.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1967,7 +1967,8 @@ ParserStatus Parser::parseDeclAttribute(DeclAttributes &Attributes, SourceLoc At
19671967

19681968
bool Parser::canParseTypeAttribute() {
19691969
TypeAttributes attrs; // ignored
1970-
return !parseTypeAttribute(attrs, /*justChecking*/ true);
1970+
return !parseTypeAttribute(attrs, /*atLoc=*/SourceLoc(),
1971+
/*justChecking*/ true);
19711972
}
19721973

19731974
/// \verbatim
@@ -1978,7 +1979,8 @@ bool Parser::canParseTypeAttribute() {
19781979
/// \param justChecking - if true, we're just checking whether we
19791980
/// canParseTypeAttribute; don't emit any diagnostics, and there's
19801981
/// no need to actually record the attribute
1981-
bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
1982+
bool Parser::parseTypeAttribute(TypeAttributes &Attributes, SourceLoc AtLoc,
1983+
bool justChecking) {
19821984
// If this not an identifier, the attribute is malformed.
19831985
if (Tok.isNot(tok::identifier) &&
19841986
// These are keywords that we accept as attribute names.
@@ -2040,7 +2042,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
20402042

20412043
// Ok, it is a valid attribute, eat it, and then process it.
20422044
StringRef Text = Tok.getText();
2043-
SourceLoc Loc = consumeToken();
2045+
consumeToken();
20442046

20452047
StringRef conventionName;
20462048
StringRef witnessMethodProtocol;
@@ -2100,7 +2102,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
21002102

21012103
// Diagnose duplicated attributes.
21022104
if (Attributes.has(attr)) {
2103-
diagnose(Loc, diag::duplicate_attribute, /*isModifier=*/false);
2105+
diagnose(AtLoc, diag::duplicate_attribute, /*isModifier=*/false);
21042106
return false;
21052107
}
21062108

@@ -2122,7 +2124,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
21222124
case TAK_callee_guaranteed:
21232125
case TAK_objc_metatype:
21242126
if (!isInSILMode()) {
2125-
diagnose(Loc, diag::only_allowed_in_sil, Text);
2127+
diagnose(AtLoc, diag::only_allowed_in_sil, Text);
21262128
return false;
21272129
}
21282130
break;
@@ -2131,27 +2133,27 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
21312133
case TAK_sil_weak:
21322134
case TAK_sil_unowned:
21332135
if (!isInSILMode()) {
2134-
diagnose(Loc, diag::only_allowed_in_sil, Text);
2136+
diagnose(AtLoc, diag::only_allowed_in_sil, Text);
21352137
return false;
21362138
}
21372139

21382140
if (Attributes.hasOwnership()) {
2139-
diagnose(Loc, diag::duplicate_attribute, /*isModifier*/false);
2141+
diagnose(AtLoc, diag::duplicate_attribute, /*isModifier*/false);
21402142
return false;
21412143
}
21422144
break;
21432145

21442146
// 'inout' attribute.
21452147
case TAK_inout:
21462148
if (!isInSILMode()) {
2147-
diagnose(Loc, diag::inout_not_attribute);
2149+
diagnose(AtLoc, diag::inout_not_attribute);
21482150
return false;
21492151
}
21502152
break;
21512153

21522154
case TAK_opened: {
21532155
if (!isInSILMode()) {
2154-
diagnose(Loc, diag::only_allowed_in_sil, "opened");
2156+
diagnose(AtLoc, diag::only_allowed_in_sil, "opened");
21552157
return false;
21562158
}
21572159

@@ -2231,7 +2233,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, bool justChecking) {
22312233
}
22322234
}
22332235

2234-
Attributes.setAttr(attr, Loc);
2236+
Attributes.setAttr(attr, AtLoc);
22352237
return false;
22362238
}
22372239

@@ -2448,8 +2450,8 @@ bool Parser::parseTypeAttributeListPresent(ParamDecl::Specifier &Specifier,
24482450
if (Attributes.AtLoc.isInvalid())
24492451
Attributes.AtLoc = Tok.getLoc();
24502452
SyntaxParsingContext AttrCtx(SyntaxContext, SyntaxKind::Attribute);
2451-
consumeToken();
2452-
if (parseTypeAttribute(Attributes))
2453+
SourceLoc AtLoc = consumeToken();
2454+
if (parseTypeAttribute(Attributes, AtLoc))
24532455
return true;
24542456
}
24552457

lib/Sema/TypeCheckType.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,8 +2040,7 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
20402040
TypeResolutionOptions options) {
20412041
// Convenience to grab the source range of a type attribute.
20422042
auto getTypeAttrRangeWithAt = [](ASTContext &ctx, SourceLoc attrLoc) {
2043-
return SourceRange(attrLoc.getAdvancedLoc(-1),
2044-
Lexer::getLocForEndOfToken(ctx.SourceMgr, attrLoc));
2043+
return SourceRange(attrLoc, attrLoc.getAdvancedLoc(1));
20452044

20462045
};
20472046

test/IDE/coloring.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,3 +454,12 @@ func acceptBuilder<T>(
454454
// CHECK: @<type>SomeBuilder</type><<type>Element</type>> label param: () -> <type>T</type>
455455
@SomeBuilder<Element> label param: () -> T
456456
) {}
457+
458+
// CHECK: <kw>func</kw> typeAttr(a: <attr-builtin>@escaping</attr-builtin> () -> <type>Int</type>) {}
459+
func typeAttr(a: @escaping () -> Int) {}
460+
461+
// CHECK: <kw>func</kw> typeAttr3(a: <attr-builtin>@ escaping</attr-builtin> () -> <type>Int</type>) {}
462+
func typeAttr3(a: @ escaping () -> Int) {}
463+
464+
// CHECK: <kw>func</kw> typeAttr2(a: @ <comment-block>/*this is fine...*/</comment-block> escaping () -> <type>Int</type>, b: <attr-builtin>@ escaping</attr-builtin> () -> <type>Int</type>) {}
465+
func typeAttr2(a: @ /*this is fine...*/ escaping () -> Int, b: @ escaping () -> Int) {}

test/IDE/structure.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,3 +305,6 @@ enum FooEnum {
305305
// CHECK: }</brace></closure></param>)</name></enum-elem></enum-case>
306306
}
307307
// CHECK: }</enum>
308+
309+
fourthCall(a: @escaping () -> Int)
310+
// CHECK: <call><name>fourthCall</name>(<arg><name>a</name>: @escaping () -> Int</arg>)</call>

test/SourceKit/InterfaceGen/gen_clang_module.swift.response

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,15 +1610,10 @@ public class FooOverlayClassDerived : Foo.FooOverlayClassBase {
16101610
key.length: 4
16111611
},
16121612
{
1613-
key.kind: source.lang.swift.syntaxtype.attribute.id,
1613+
key.kind: source.lang.swift.syntaxtype.attribute.builtin,
16141614
key.offset: 2465,
16151615
key.length: 11
16161616
},
1617-
{
1618-
key.kind: source.lang.swift.syntaxtype.attribute.builtin,
1619-
key.offset: 2466,
1620-
key.length: 10
1621-
},
16221617
{
16231618
key.kind: source.lang.swift.syntaxtype.identifier,
16241619
key.offset: 2477,

0 commit comments

Comments
 (0)