-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Parser: calculating interface hash only for primary files. #19196
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
Conversation
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad it's looking easy! I think I'd prefer a different implementation strategy though…it feels like you mixed two of my ideas.
include/swift/AST/Module.h
Outdated
@@ -1117,20 +1118,23 @@ class SourceFile final : public FileUnit { | |||
/// Set the root refinement context for the file. | |||
void setTypeRefinementContext(TypeRefinementContext *TRC); | |||
|
|||
void initInterfaceHashIfEmpty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right to me. I'd expect something like enableInterfaceHash()
, which would assert if it were already enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense.
include/swift/Subsystems.h
Outdated
@@ -117,6 +117,7 @@ namespace swift { | |||
/// | |||
/// \return true if the parser found code with side effects. | |||
bool parseIntoSourceFile(SourceFile &SF, unsigned BufferID, bool *Done, | |||
bool CollectInterfaceHash, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…and then you wouldn't need this.
c4513ff
to
0b3b52b
Compare
@swift-ci please smoke test |
lib/Frontend/Frontend.cpp
Outdated
@@ -745,7 +745,8 @@ void CompilerInstance::parseLibraryFile( | |||
auto &Diags = NextInput->getASTContext().Diags; | |||
auto DidSuppressWarnings = Diags.getSuppressWarnings(); | |||
Diags.setSuppressWarnings(DidSuppressWarnings || !IsPrimary); | |||
|
|||
if (IsPrimary) | |||
NextInput->enableInterfaceHash(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can simplify this by doing it even earlier, in CompilerInstance::createSourceFileForMainModule
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put it in recordPrimarySourceFile
.
0b3b52b
to
fe0046b
Compare
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that's a nice, small diff! Thanks for working on this, Xi. Hopefully it still gives you the speedups you need. (And it'll be something you can refine later, I guess.)
@swift-ci please smoke test OS X Platform |
1 similar comment
@swift-ci please smoke test OS X Platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Xi! This patch itself may speedups non-primary files parsing.
@swift-ci please smoke test |
Per discussion we had in #19192, we only need to collect interface hashes for primary files. This change allows parser to consume tokens out of order for non-primary files.