Skip to content

[SILGen] Add flag to skip typechecking and SIL gen for all function bodies #34489

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

Jump to bottom
Merged
merged 1 commit into from
Nov 6, 2020
Merged

[SILGen] Add flag to skip typechecking and SIL gen for all function bodies #34489

merged 1 commit into from
Nov 6, 2020

Conversation

bnbarham
Copy link
Contributor

Adds a new flag "-experimental-skip-all-function-bodies" that skips SIL
generation entirely. Function bodies will not be type checked, but may
still be parsed if another flag forces delayed parsing off.

@bnbarham bnbarham requested a review from akyrtzi October 29, 2020 05:14
@bnbarham
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ae785fc10c68331b6afd35a6b874f20023c503f5

harlanhaskins
harlanhaskins reviewed Oct 29, 2020
View reviewed changes
@bnbarham bnbarham requested review from nkcsgexi and xymus October 29, 2020 21:32
@slavapestov
Copy link
Contributor

I think these days we don't have a flag for delayed parsing anymore, we just always lazily parse function bodies in non-primary files the first time you call AbstractFunctionDecl::getBody(). So if nothing asks for the body of a function it won't be parsed.

@akyrtzi
Copy link
Contributor

akyrtzi commented Oct 30, 2020

The test seems to only focus on checking SIL generation but there doesn't seem to be a test to check that typechecking is really avoided.
I'd suggest to add some test cases to check for that, see uses of -Xfrontend -debug-forbid-typecheck-prefix in other tests to see how you could accomplish this.

@bnbarham
Copy link
Contributor Author

bnbarham commented Oct 30, 2020 •
edited
Loading

I think these days we don't have a flag for delayed parsing anymore, we just always lazily parse function bodies in non-primary files the first time you call AbstractFunctionDecl::getBody(). So if nothing asks for the body of a function it won't be parsed.

Sorry, may not have been clear in my description there. By "another flag" I was mostly referring to eg. "-dump-parse", "-verify-syntax-tree", etc. Basically, all the conditions in hasDelayedBodyParsing.

The specific case I saw was the stdlib being built with "-verify-syntax-tree", which disables delayed parsing. Previous to noticing that I had actually changed setBodySkipped to not include BodyKind::Parsed, as well as having removed getInnermostSkippedFunctionContext and associated code. Had to put that back in though :)

@bnbarham
Copy link
Contributor Author

The test seems to only focus on checking SIL generation but there doesn't seem to be a test to check that typechecking is really avoided.
I'd suggest to add some test cases to check for that, see uses of -Xfrontend -debug-forbid-typecheck-prefix in other tests to see how you could accomplish this.

Will do, thanks.

@bnbarham bnbarham force-pushed the benb/skip-all-function-bodies branch from ae785fc to a28ba39 Compare November 2, 2020 03:32
@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 2, 2020

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Nov 2, 2020

Build failed
Swift Test Linux Platform
Git Sha - a28ba3928d357c992ef886401f5cb64cc4b95863

@bnbarham bnbarham force-pushed the benb/skip-all-function-bodies branch from a28ba39 to 0d5ac1e Compare November 2, 2020 23:26
@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 2, 2020

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Nov 2, 2020

Build failed
Swift Test Linux Platform
Git Sha - 0d5ac1e7f9e9b8f3a2e9c1576ff92ed267746c32

xymus
xymus approved these changes Nov 3, 2020
View reviewed changes
@bnbarham bnbarham force-pushed the benb/skip-all-function-bodies branch from 0d5ac1e to 75906c9 Compare November 4, 2020 01:46
@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 4, 2020

@swift-ci please test Linux platform

@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 4, 2020

Okay... well, I'm going to leave the getSourceRange change out for now. It was previously needed but isn't with the current changes, so it should have just been a small cleanup (apparently not though). The only case I could see it being different is when a Synthesized accessor has a start location, but when would that be the case?

I'll collapse the two commits and re-run the tests.

@bnbarham bnbarham force-pushed the benb/skip-all-function-bodies branch from 75906c9 to b76b6f3 Compare November 4, 2020 05:00
@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 4, 2020

@swift-ci please smoke test

@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 4, 2020

@swift-ci please smoke test OS X platform

@bnbarham bnbarham force-pushed the benb/skip-all-function-bodies branch from b76b6f3 to 3598ceb Compare November 5, 2020 01:32
@bnbarham bnbarham changed the title [SILGen] Add flag to skip SIL generation for all function bodies [SILGen] Add flag to skip typechecking and SIL gen for all function bodies Nov 5, 2020
@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 5, 2020

Going to wait for #34569 to be merged. I'll then rebase and add those new tests into the new combined file.

@bnbarham bnbarham force-pushed the benb/skip-all-function-bodies branch from 3598ceb to de908d8 Compare November 5, 2020 21:08
@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 5, 2020

@swift-ci please test

@bnbarham
[SILGen] Add flag to skip typechecking and SIL gen for function bodies
7cee600 
Adds a new flag "-experimental-skip-all-function-bodies" that skips
typechecking and SIL generation for all function bodies (where
possible).

`didSet` functions are still typechecked and have SIL generated as their
body is checked for the `oldValue` parameter, but are not serialized.
Parsing will generally be skipped as well, but this isn't necessarily
the case since other flags (eg. "-verify-syntax-tree") may force delayed
parsing off.
@bnbarham bnbarham force-pushed the benb/skip-all-function-bodies branch from de908d8 to 7cee600 Compare November 6, 2020 02:08
@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 6, 2020

@swift-ci please smoke test

@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 6, 2020

Forgot to run clang-format, but otherwise the same.

@bnbarham
Copy link
Contributor Author

bnbarham commented Nov 6, 2020

Windows error is unrelated (compile error in llvm-project) and it had passed previous to the clang-format, so just going to merge.

@bnbarham bnbarham merged commit fdff522 into swiftlang:main Nov 6, 2020
@bnbarham bnbarham deleted the benb/skip-all-function-bodies branch November 6, 2020 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov left review comments

@harlanhaskins harlanhaskins harlanhaskins left review comments

@akyrtzi akyrtzi akyrtzi approved these changes

@xymus xymus xymus approved these changes

@nkcsgexi nkcsgexi Awaiting requested review from nkcsgexi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bnbarham @swift-ci @slavapestov @akyrtzi @xymus @harlanhaskins