[ParseableInterface] Serialize module into buffer and load from it #23173
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
harlanhaskins
force-pushed
the
never-trust-the-system
branch
2 times, most recently
from
789b52f to
c3b8f15
Compare
Contributor
|
Really amazing comments in this patch! |
harlanhaskins
force-pushed
the
never-trust-the-system
branch
from
c3b8f15 to
43ca89a
Compare
Contributor
Author
harlanhaskins
force-pushed
the
never-trust-the-system
branch
4 times, most recently
from
68009f7 to
75515ff
Compare
harlanhaskins
force-pushed
the
never-trust-the-system
branch
from
75515ff to
71eb4f1
Compare
harlanhaskins
changed the title
Contributor
Author
|
@swift-ci please smoke test |
jrose-apple
reviewed
Mar 21, 2019
Contributor
jrose-apple
left a comment
jrose-apple
left a comment
There was a problem hiding this comment.
I like the safety of std::unique_ptr a lot here. Ownership FTW!
jrose-apple
reviewed
Mar 21, 2019
jrose-apple
approved these changes
Mar 21, 2019
benlangmuir
approved these changes
Mar 21, 2019
Contributor
benlangmuir
left a comment
benlangmuir
left a comment
There was a problem hiding this comment.
LGTM, thanks Harlan!
In addition to being wasteful, this is a correctness issue -- the compiler should only ever have one view of this file, and it should not read a potentially different file after validating dependencies. rdar://48654608
harlanhaskins
force-pushed
the
never-trust-the-system
branch
from
481ff9c to
7bdf542
Compare
Contributor
Author
|
@swift-ci please test |
Contributor
|
Build failed |
Contributor
Author
|
@jrose-apple Are these ClangImporter changes okay? |
Contributor
Author
|
@swift-ci please smoke test Linux |
jrose-apple
reviewed
Mar 22, 2019
harlanhaskins
force-pushed
the
never-trust-the-system
branch
from
5d897b8 to
18aee59
Compare
Contributor
Author
|
@swift-ci please smoke test Linux |
Contributor
Author
|
@swift-ci please smoke test macOS |
Contributor
Author
|
Seems like the Linux bot had issues. Let's try again. @swift-ci please smoke test Linux |
harlanhaskins
force-pushed
the
never-trust-the-system
branch
from
18aee59 to
7e22618
Compare
added 2 commits
March 22, 2019 17:29
Previously, we would assert that there's a runtime resource dir available, but then accept the possibility of no glibc.modulemap. We should just do the same thing for 'no resource dir' as 'no module map'.
harlanhaskins
force-pushed
the
never-trust-the-system
branch
from
7e22618 to
5a29cdc
Compare
jrose-apple
approved these changes
Mar 23, 2019
Contributor
Author
|
@swift-ci please test and merge |
Contributor
Author
|
@swift-ci please test |
Contributor
|
Build failed |
Contributor
|
Build failed |
harlanhaskins
pushed a commit
to harlanhaskins/swift
that referenced
this pull request
Mar 23, 2019
…-system [ParseableInterface] Serialize module into buffer and load from it
harlanhaskins
pushed a commit
to harlanhaskins/swift
that referenced
this pull request
Mar 23, 2019
…-system [ParseableInterface] Serialize module into buffer and load from it
This was referenced Mar 23, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In addition to being wasteful, this is a correctness issue -- the
compiler should only ever have one view of this file, and it should not
read a potentially different file after validating dependencies.
rdar://48654608