Skip to content

[Importer] Wrap the VFS passed to Clang with an overlay FS instead #41546

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 1 commit into from
Feb 25, 2022

Conversation

bnbarham
Copy link
Contributor

OverlayFileSystem is very simple - it just passes along each request
to each VFS it contains until one is successful (or none are). Use it
when wrapping the VFS to pass down into the Clang invocation creation,
instead of the much more complicated RedirectingFileSystem.

This has the side effect of also fixing a case where due to a bug in
RedirectingFileSystem, the originally passed in path will be returned
regardless of use-external-name. While that should also be fixed,
there is no reason to use this VFS here anyway.

Added a small cursor info test case that should catch issues like this
in the future.

@bnbarham
Copy link
Contributor Author

@swift-ci please test

@bnbarham bnbarham requested a review from beccadax February 24, 2022 20:17
@bnbarham
Copy link
Contributor Author

Whoops, would help if I tested it with the LLVM change that it requires!

@bnbarham
Copy link
Contributor Author

swiftlang/llvm-project#3990

@swift-ci please test

Copy link
Contributor

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

Makes sense. LGTM

@bnbarham
Copy link
Contributor Author

@swift-ci please test

`OverlayFileSystem` is very simple - it just passes along each request
to each VFS it contains until one is successful (or none are). Use it
when wrapping the VFS to pass down into the Clang invocation creation,
instead of the much more complicated `RedirectingFileSystem`.

This has the side effect of also fixing a case where due to a bug in
`RedirectingFileSystem`, the originally passed in path will be returned
regardless of `use-external-name`. While that should also be fixed,
there is no reason to use this VFS here anyway.

Added a small cursor info test case that should catch issues like this
in the future.
@bnbarham bnbarham force-pushed the use-overlay-fs-instead branch from 93cf869 to bbc98aa Compare February 25, 2022 18:06
@bnbarham
Copy link
Contributor Author

Good old Windows paths. They get me every time!

@bnbarham
Copy link
Contributor Author

@swift-ci please test

@bnbarham bnbarham merged commit b2ed8a2 into swiftlang:main Feb 25, 2022
@bnbarham bnbarham deleted the use-overlay-fs-instead branch February 25, 2022 21:40
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