Skip to content

[libSyntax] Add a unsafe but fast SyntaxDataRef version of SyntaxData #36250

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

Closed
wants to merge 3 commits into from
Closed

[libSyntax] Add a unsafe but fast SyntaxDataRef version of SyntaxData #36250

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
56a9234
[libSyntax] Reference count SyntaxData
ahoppen Feb 25, 2021
996e893
[libSyntax] Add a unsafe but fast SyntaxDataRef version of SyntaxData
ahoppen Feb 26, 2021
d386970
[libSyntax] Model IsRef in SyntaxDataRef by a a virtual isRef method
ahoppen Mar 4, 2021
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
121 changes: 114 additions & 7 deletions include/swift/Syntax/AbsoluteRawSyntax.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,37 +218,144 @@ struct AbsoluteRawSyntax {
const AbsoluteSyntaxInfo Info;

public:
/// Create a null \c AbsoluteRawSyntax to which a real \c AbsoluteRawSyntax
/// can be stored later.
explicit AbsoluteRawSyntax()
: Raw(nullptr), Info(AbsoluteSyntaxPosition(0, 0),
SyntaxIdentifier(0, SyntaxIndexInTree::zero())) {}

/// Create a new \c AbsoluteRawData backed by \p Raw and with additional \p
/// Info. The caller of this constructor is responsible to ensure that the
/// Arena of \p Raw (and thus \p Raw itself) outlives this \c
/// AbsoluteRawSyntax.
AbsoluteRawSyntax(const RawSyntax *Raw, AbsoluteSyntaxInfo Info)
: Raw(Raw), Info(Info) {}
: Raw(Raw), Info(Info) {
assert(Raw != nullptr &&
"A AbsoluteRawSyntax created through the memberwise constructor "
"should always have a RawSyntax");
}

/// Whether this is a null \c AbsoluteRawSyntax created through the default
/// constructor.
bool isNull() const { return Raw == nullptr; }

/// Construct a \c AbsoluteRawSyntax for a \c RawSyntax node that represents
/// the syntax tree's root.
static AbsoluteRawSyntax forRoot(const RawSyntax *Raw) {
return AbsoluteRawSyntax(Raw, AbsoluteSyntaxInfo::forRoot());
}

const RawSyntax *getRaw() const { return Raw; }
const RawSyntax *getRaw() const {
assert(!isNull() && "Cannot get Raw of a null AbsoluteRawSyntax");
return Raw;
}

AbsoluteSyntaxInfo getInfo() const { return Info; }
AbsoluteSyntaxInfo getInfo() const {
assert(!isNull() && "Cannot get Raw of a null AbsoluteRawSyntax");
return Info;
}

/// Get the position at which the leading triva of this node starts.
AbsoluteSyntaxPosition getPosition() const { return Info.getPosition(); };
AbsoluteSyntaxPosition getPosition() const {
return getInfo().getPosition();
};

SyntaxIdentifier getNodeId() const { return Info.getNodeId(); };
SyntaxIdentifier getNodeId() const { return getInfo().getNodeId(); };

AbsoluteSyntaxPosition::IndexInParentType getIndexInParent() const {
return getPosition().getIndexInParent();
}

size_t getNumChildren() const { return getRaw()->getLayout().size(); }

/// Get the child at \p Index if it exists. If the node does not have a child
/// at \p Index, return \c None. Asserts that \p Index < \c NumChildren
Optional<AbsoluteRawSyntax>
getChild(AbsoluteSyntaxPosition::IndexInParentType Index) const {
Copy link
Member

@rintaro rintaro Mar 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can we return isNull() AbsoluteRawSyntax instead of Optional? If not, what the purpose of isNull()?

Copy link
Member Author

Choose a reason for hiding this comment

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

The purpose of the isNull() AbsoluteRawSyntax is so we can construct a null SyntaxDataRef (which has a AbsoluteRawSyntax member). We need this for the getChildRef method, I already described here, which is used as follows:

SyntaxRef result;
someNode.getChildRef(1, result);

Technically, result just needs to be uninitialized memory. If you know how to create uninitialized stack memory, we can remove isNull altogether.

I though about returning a null AbsoluteRawSyntax here, but decided against it, because I expect there will be a whole category of bugs where we would be accessing an optional child without check if it is null.

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m looking into whether we can make Optional<AbsoluteRawSyntax> a zero-cost wrapper around AbsoluteRawSyntax whose null value is internally backed by isNull() and make isNull() private. Then we would have the compile time safety of Optional but the runtime behavior as if we returned an isNull() AbsoluteRawSyntax.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the isNull function and made Optional<AbsoluteRawSyntax> a zero-cost wrapper. Since the change wasn't trivial (~250 LOC), I created a follow-up PR for it here.

assert(Index < getNumChildren() && "Index out of bounds");
auto RawChild = getRaw()->getChild(Index);
if (RawChild) {
return getPresentChild(Index);
} else {
return None;
}
}

/// Get the child at \p Index, asserting that it exists. This is slightly
/// more performant than \c getChild in these cases since the \c
/// AbsoluteRawSyntax node does not have to be wrapped in an \c Optional.
AbsoluteRawSyntax
getPresentChild(AbsoluteSyntaxPosition::IndexInParentType Index) const {
assert(Index < getNumChildren() && "Index out of bounds");
auto RawChild = getRaw()->getChild(Index);
assert(RawChild &&
"Child retrieved using getPresentChild must always exist");

AbsoluteSyntaxPosition Position = getPosition().advancedToFirstChild();
SyntaxIdentifier NodeId = getNodeId().advancedToFirstChild();

for (size_t I = 0; I < Index; ++I) {
Position = Position.advancedBy(getRaw()->getChild(I));
NodeId = NodeId.advancedBy(getRaw()->getChild(I));
}

AbsoluteSyntaxInfo Info(Position, NodeId);
return AbsoluteRawSyntax(RawChild, Info);
}

/// Get the first non-missing token node in this tree. Return \c None if
/// this node does not contain non-missing tokens.
Optional<AbsoluteRawSyntax> getFirstToken() const {
if (getRaw()->isToken() && !getRaw()->isMissing()) {
return *this;
}

size_t NumChildren = getNumChildren();
for (size_t I = 0; I < NumChildren; ++I) {
if (auto Child = getChild(I)) {
if (Child->getRaw()->isMissing()) {
continue;
}

if (auto Token = Child->getFirstToken()) {
return Token;
}
}
}
return None;
}

/// Get the last non-missing token node in this tree. Return \c None if
/// this node does not contain non-missing tokens.
Optional<AbsoluteRawSyntax> getLastToken() const {
if (getRaw()->isToken() && !getRaw()->isMissing()) {
return *this;
}

for (int I = getNumChildren() - 1; I >= 0; --I) {
if (auto Child = getChild(I)) {
if (Child->getRaw()->isMissing()) {
continue;
}

if (auto Token = Child->getLastToken()) {
return Token;
}
}
}
return None;
}

/// Construct a new \c AbsoluteRawSyntax node that has the same info as the
/// current one, but
/// - the \p NewRaw as the backing storage
/// - the \p NewRootId as the RootId
AbsoluteRawSyntax
replacingSelf(const RawSyntax *NewRaw,
SyntaxIdentifier::RootIdType NewRootId) const {
SyntaxIdentifier NewNodeId(NewRootId, Info.getNodeId().getIndexInTree());
AbsoluteSyntaxInfo NewInfo(Info.getPosition(), NewNodeId);
SyntaxIdentifier NewNodeId(NewRootId,
getInfo().getNodeId().getIndexInTree());
AbsoluteSyntaxInfo NewInfo(getInfo().getPosition(), NewNodeId);
return AbsoluteRawSyntax(NewRaw, NewInfo);
}
};
Expand Down
1 change: 1 addition & 0 deletions include/swift/Syntax/RawSyntax.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ class RawSyntax final
/// Get a child based on a particular node's "Cursor", indicating
/// the position of the terms in the production of the Swift grammar.
const RawSyntax *getChild(CursorIndex Index) const {
assert(Index < getNumChildren() && "Index out of bounds");
return getLayout()[Index];
}

Expand Down
22 changes: 10 additions & 12 deletions include/swift/Syntax/Syntax.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ class Syntax {
friend struct SyntaxFactory;

protected:
SyntaxData Data;
RC<const SyntaxData> Data;

public:
explicit Syntax(const SyntaxData Data) : Data(Data) {}
explicit Syntax(const RC<const SyntaxData> &Data) : Data(Data) {}

virtual ~Syntax() {}

Expand All @@ -86,9 +86,7 @@ class Syntax {
}

/// Get the Data for this Syntax node.
const SyntaxData &getData() const {
return Data;
}
const RC<const SyntaxData> &getData() const { return Data; }

/// Cast this Syntax node to a more specific type, asserting it's of the
/// right kind.
Expand All @@ -113,7 +111,7 @@ class Syntax {

/// Returns the child index of this node in its parent,
/// if it has one, otherwise 0.
CursorIndex getIndexInParent() const { return getData().getIndexInParent(); }
CursorIndex getIndexInParent() const { return getData()->getIndexInParent(); }

/// Return the number of bytes this node takes when spelled out in the source
size_t getTextLength() const { return getRaw()->getTextLength(); }
Expand Down Expand Up @@ -157,8 +155,8 @@ class Syntax {
SWIFT_DEBUG_DUMP;

bool hasSameIdentityAs(const Syntax &Other) const {
return Data.getAbsoluteRaw().getNodeId() ==
Other.Data.getAbsoluteRaw().getNodeId();
return Data->getAbsoluteRaw().getNodeId() ==
Other.Data->getAbsoluteRaw().getNodeId();
}

static bool kindof(SyntaxKind Kind) {
Expand All @@ -180,23 +178,23 @@ class Syntax {

/// Get the offset at which the leading trivia of this node starts.
AbsoluteOffsetPosition getAbsolutePositionBeforeLeadingTrivia() const {
return Data.getAbsolutePositionBeforeLeadingTrivia();
return Data->getAbsolutePositionBeforeLeadingTrivia();
}

/// Get the offset at which the actual content (i.e. non-triva) of this node
/// starts.
AbsoluteOffsetPosition getAbsolutePositionAfterLeadingTrivia() const {
return Data.getAbsolutePositionAfterLeadingTrivia();
return Data->getAbsolutePositionAfterLeadingTrivia();
}

/// Get the offset at which the trailing trivia of this node starts.
AbsoluteOffsetPosition getAbsoluteEndPositionBeforeTrailingTrivia() const {
return Data.getAbsoluteEndPositionBeforeTrailingTrivia();
return Data->getAbsoluteEndPositionBeforeTrailingTrivia();
}

/// Get the offset at which the trailing trivia of this node starts.
AbsoluteOffsetPosition getAbsoluteEndPositionAfterTrailingTrivia() const {
return Data.getAbsoluteEndPositionAfterTrailingTrivia();
return Data->getAbsoluteEndPositionAfterTrailingTrivia();
}

// TODO: hasSameStructureAs ?
Expand Down
22 changes: 11 additions & 11 deletions include/swift/Syntax/SyntaxCollection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ class SyntaxCollection : public Syntax {
friend class Syntax;

private:
static SyntaxData makeData(std::initializer_list<Element> &Elements,
const RC<SyntaxArena> &Arena) {
static RC<const SyntaxData> makeData(std::initializer_list<Element> &Elements,
const RC<SyntaxArena> &Arena) {
std::vector<const RawSyntax *> List;
List.reserve(Elements.size());
for (auto &Elt : Elements)
Expand All @@ -69,7 +69,7 @@ class SyntaxCollection : public Syntax {
}

public:
SyntaxCollection(const SyntaxData Data) : Syntax(Data) {}
SyntaxCollection(const RC<const SyntaxData> &Data) : Syntax(Data) {}

SyntaxCollection(std::initializer_list<Element> list):
SyntaxCollection(SyntaxCollection::makeData(list)) {}
Expand Down Expand Up @@ -105,7 +105,7 @@ class SyntaxCollection : public Syntax {
Element operator[](const size_t Index) const {
assert(Index < size());
assert(!empty());
return Element(*Data.getChild(Index));
return Element(Data->getChild(Index));
}

/// Return a new collection with the given element added to the end.
Expand All @@ -119,7 +119,7 @@ class SyntaxCollection : public Syntax {
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
getRaw()->getPresence(),
getRaw()->getArena());
return SyntaxCollection<CollectionKind, Element>(Data.replacingSelf(Raw));
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
}

/// Return a new collection with an element removed from the end.
Expand All @@ -131,7 +131,7 @@ class SyntaxCollection : public Syntax {
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
getRaw()->getPresence(),
getRaw()->getArena());
return SyntaxCollection<CollectionKind, Element>(Data.replacingSelf(Raw));
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
}

/// Return a new collection with the given element appended to the front.
Expand All @@ -144,7 +144,7 @@ class SyntaxCollection : public Syntax {
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
getRaw()->getPresence(),
getRaw()->getArena());
return SyntaxCollection<CollectionKind, Element>(Data.replacingSelf(Raw));
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
}

/// Return a new collection with an element removed from the end.
Expand All @@ -156,7 +156,7 @@ class SyntaxCollection : public Syntax {
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
getRaw()->getPresence(),
getRaw()->getArena());
return SyntaxCollection<CollectionKind, Element>(Data.replacingSelf(Raw));
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
}

/// Return a new collection with the Element inserted at index i.
Expand All @@ -177,7 +177,7 @@ class SyntaxCollection : public Syntax {
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
getRaw()->getPresence(),
getRaw()->getArena());
return SyntaxCollection<CollectionKind, Element>(Data.replacingSelf(Raw));
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
}

/// Return a new collection with the element removed at index i.
Expand All @@ -190,14 +190,14 @@ class SyntaxCollection : public Syntax {
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
getRaw()->getPresence(),
getRaw()->getArena());
return SyntaxCollection<CollectionKind, Element>(Data.replacingSelf(Raw));
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
}

/// Return an empty syntax collection of this type.
SyntaxCollection<CollectionKind, Element> cleared() const {
auto Raw = RawSyntax::makeAndCalcLength(
CollectionKind, {}, getRaw()->getPresence(), getRaw()->getArena());
return SyntaxCollection<CollectionKind, Element>(Data.replacingSelf(Raw));
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
}

static bool kindof(SyntaxKind Kind) {
Expand Down
Loading