Skip to content

[ASTGen] Add experimental feature to use ASTGen in lieu of parsing types #66033

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
merged 11 commits into from
May 24, 2023

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented May 19, 2023

Introduce a new experimental feature ASTGenTypes that uses ASTGen to translate the Swift syntax tree (produced by the new Swift parser) into C++ TypeRepr nodes instead of having the C++ parser create the nodes.

The approach here is to intercept the C++ parser's parseType operation to find the Swift syntax node at the given position (where the lexer currently is) and have ASTGen translate that into the corresponding C++ AST node. Then, we spin the lexer forward to the token immediately following the end of the syntax node and continue parsing.

Introduce a collection of small fixes to the mapping of type syntax nodes into TypeReprs:

  • Fix crash due to generic arguments having the wrong pointer values (silly enums)
  • Support specifiers like inout, consuming, and isolated
  • Recognize Any and treat it as the empty composition type
  • Handle back-ticked identifiers
  • Handle variadic parameters in functions and function types
  • Handle simple type attributes (this is not complete)
  • Handle protocol composition types and provide correct source information
  • Handle _ identifier as the empty identifier

With all of these changes, a hacked compiler that enables this experimental flag all the time successfully handles 236 of 245 tests in test/Sema.

@DougGregor
Copy link
Member Author

@CodaFi you're gonna like this...

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

Copy link
Contributor

@CodaFi CodaFi left a comment

Choose a reason for hiding this comment

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

I do! I do like this!

/// is NULL, something went wrong.
template<typename T>
using ASTFromSyntaxTreeCallback = std::pair<T*, const void *>(
void *sourceFile, const void *sourceLoc
Copy link
Member

Choose a reason for hiding this comment

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

We really need to figure out a better way to pass things around than using void *.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is pretty dreadful. Typedefs would improve on things a little bit. @bnbarham keeps threatening to add them

Copy link
Contributor

Choose a reason for hiding this comment

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

#66044 for the common types (though this would be the first SourceFile I think?). Decls/exprs/typerefs/etc are a little more difficult since the visitor just returns a ASTNode for each visit at the moment. Even disregarding that I'm not sure we want to bridge every single AST node type that we need.

But it would be nice to do something to avoid:

sourceFilePtr: UnsafeRawPointer,
sourceLocationPtr: UnsafePointer<UInt8>?,
type: Node.Type
type: Node.Type,
wantOutermost: Bool = false
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment what, wantOutermost does? Also, I think it would also be good to document the other parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I will. I should really move this API somewhere else (e.g., on ExportedSourceFile) because it's in an odd place now that I made it non-private.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

DougGregor added 11 commits May 22, 2023 21:39
Introduce a new experimental feature `ASTGenTypes` that uses ASTGen to
translate the Swift syntax tree (produced by the new Swift parser)
into C++ `TypeRepr` nodes instead of having the C++ parser create the
nodes.

The approach here is to intercept the C++ parser's `parseType`
operation to find the Swift syntax node at the given position (where
the lexer currently is) and have ASTGen translate that into the
corresponding C++ AST node. Then, we spin the lexer forward to the
token immediately following the end of the syntax node and continue
parsing.
The pointers we were vending to C++ had low bits set because they were
AST node entries rather than the raw TypeReprs.
There is a modeling difference between the swift-syntax tree and the
C++ type representation (TypeRepr) that is a little odd here, so we
end up parsing the ellipsis on the C++ side rather than looking "up"
the syntax tree to find it.
Bridge the simple type attributes like `@autoclosure` into an
`AttributedTypeRepr`. We don't validate the arguments, and we don't
handle more complicated attributes like `@convention(c)` just yet.
Protocol composition types such as `P & Q` are similar to nested types
like `A.B` because the innermost type syntax node at the given position
doesn't cover the whole type. Adjust by looking back up the tree.
This all feels like a hack, and there should be a better way.

While here, fix the source ranges passed in to create
`CompositionTypeRepr`. We were tripping assertions due to missing
source-location information.
Instead of "spinning" the C++ lexer, consuming tokens uptil we get past
the point where ASTGen told us the end of the syntax node was, just
reset the lexer to exactly that position. This is more efficient, but
also fixes problems where we would end up skipping past a `>` that had
been split.
@DougGregor DougGregor force-pushed the parse-types-via-astgen branch from 9dcfbf2 to 7224b81 Compare May 23, 2023 05:43
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

I'm going to merge this and we'll iterate to get it into a nicer form before writing more parsing logic

@DougGregor DougGregor merged commit c5ccf11 into swiftlang:main May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants