Skip to content

Formalize some SourceFile parsing outputs #32161

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 9 commits into from
Jun 8, 2020

Conversation

hamishknight
Copy link
Contributor

Currently when parsing a SourceFile, the parser gets handed pointers so that it can write the interface hash and collected tokens directly into the file. It can also call setSyntaxRoot at the end of parsing to set the syntax tree.

In preparation for the removal of performParseOnly, this PR formalizes these values as outputs of ParseSourceFileRequest, ensuring that the file gets parsed when the interface hash, collected tokens, or syntax tree is queried.

In addition, it cleans up SourceFile a little by sinking a couple of parsing options into SourceFile::ParsingFlags.

And fix the name of the underlying vector.
Now that SIL files no longer interleave parsing
with type-checking, the query doesn't make much
sense. Inline it into its only client,
`shouldBuildSyntaxTree`.
Sink the `BuildSyntaxTree` and
`CollectParsedTokens` bits into
`SourceFile::ParsingFlags`, with a static method
to get the parsing options from the lang opts.

Also add a parsing flag for enabling the interface
hash, which can be used instead of calling
`enableInterfaceHash`.
The `SaveAndRestore` is unnecessary as `Parser`'s
constructor already sets up the interface hash,
and the request covers the `FrontendStatsTracer`.
Rename `Bag` to `Tokens`, and query the
SourceManager from the ASTContext instead of
storing it directly.
Currently when parsing a SourceFile, the parser
gets handed pointers so that it can write the
interface hash and collected tokens directly into
the file. It can also call `setSyntaxRoot` at
the end of parsing to set the syntax tree.

In preparation for the removal of
`performParseOnly`, this commit formalizes these
values as outputs of `ParseSourceFileRequest`,
ensuring that the file gets parsed when the
interface hash, collected tokens, or syntax tree
is queried.
This is temporarily swapped in as the token
receiver while backtracking, so make sure we don't
try to call `finalize` on it.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@swift-ci please test source compatibility

@hamishknight hamishknight requested review from rintaro and CodaFi and removed request for rintaro June 3, 2020 18:05
/// Retrieve the token receiver from the parser once it has finished parsing.
std::unique_ptr<ConsumeTokenReceiver> takeTokenReceiver() {
assert(Tok.is(tok::eof) && "not done parsing yet");
auto *receiver = TokReceiver;
Copy link
Contributor

Choose a reason for hiding this comment

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

@rintaro Does it make sense to model TokReceiver as a std::unique_ptr as well? It would make the ownership transfer here a little cleaner to express.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about that too, unfortunately we currently swap in a pointer to a DelayedTokenReceiver on the stack while backtracking.

Copy link
Contributor

Choose a reason for hiding this comment

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

😢

@swiftlang swiftlang deleted a comment from swift-ci Jun 3, 2020
@swiftlang swiftlang deleted a comment from swift-ci Jun 3, 2020
@swiftlang swiftlang deleted a comment from swift-ci Jun 3, 2020
@swiftlang swiftlang deleted a comment from swift-ci Jun 3, 2020
@swiftlang swiftlang deleted a comment from swift-ci Jun 3, 2020
@swiftlang swiftlang deleted a comment from swift-ci Jun 3, 2020
@swiftlang swiftlang deleted a comment from swift-ci Jun 3, 2020
A previous commit inadvertently changed the
logic such that the member hash of an extension
body would be set to a partial interface hash.
Luckily this shouldn't have caused any behavioural
change as the interface hash itself would have
been left unaffected.

This commit makes sure we preserve the original
behaviour where if we don't have the body tokens
hashed separately, we give the body hash a default
constructed MD5.

Noticed by inspection.
Use `shouldBuildSyntaxTree` instead.
@hamishknight
Copy link
Contributor Author

swiftlang/llvm-project#1307

@swift-ci please test

@swiftlang swiftlang deleted a comment from swift-ci Jun 8, 2020
@hamishknight
Copy link
Contributor Author

Merging so I can make further progress – @rintaro I'm more than happy to address any review feedback in a follow-up.

@hamishknight hamishknight merged commit 81483cc into swiftlang:master Jun 8, 2020
@hamishknight hamishknight deleted the pipeline-parse branch June 8, 2020 17:56
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.

2 participants