Skip to content

Enable index store based on Clang feature detection #7287

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 5, 2024

Conversation

neonichu
Copy link
Contributor

Today, the index store is only enabled on Darwin by default and needs a manual opt-in on other platforms. We can instead switch this to enabling it based on whether the used clang supports -index-store-path.

rdar://117744039

@neonichu neonichu self-assigned this Jan 23, 2024
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

Not really sure if we should land this in the current form, it does make builds on macOS slightly slower for no good reason (since we're always doing the feature detection through shelling out to clang).

I see two paths forward:

  • we find a better way to do the feature detection that is acceptable on macOS as well
  • we still enable unconditionally on macOS, but that makes the test useless (which is fine in principle, we can delete it)

@neonichu neonichu force-pushed the index-store-based-on-clang branch from cdd1629 to 687b615 Compare February 1, 2024 21:11
@neonichu neonichu requested a review from bnbarham as a code owner February 1, 2024 21:11
@neonichu
Copy link
Contributor Author

neonichu commented Feb 1, 2024

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Feb 1, 2024

@swift-ci please test windows

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Is there any way to disable the index if it isn't wanted?

@neonichu neonichu force-pushed the index-store-based-on-clang branch 2 times, most recently from 9177b8f to 3567d6a Compare February 2, 2024 23:23
@neonichu
Copy link
Contributor Author

neonichu commented Feb 2, 2024

Is there any way to disable the index if it isn't wanted?

Not right now, do you should think that should be possible?

@neonichu
Copy link
Contributor Author

neonichu commented Feb 2, 2024

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Feb 2, 2024

@swift-ci please test windows

1 similar comment
@neonichu
Copy link
Contributor Author

neonichu commented Feb 2, 2024

@swift-ci please test windows

@neonichu
Copy link
Contributor Author

neonichu commented Feb 2, 2024

@swift-ci please test macOS

@bnbarham
Copy link
Contributor

bnbarham commented Feb 3, 2024

Not right now, do you should think that should be possible?

Yes. There's likely an issue for it... somewhere.

@neonichu
Copy link
Contributor Author

neonichu commented Feb 5, 2024

Actually, we do have --disable-index-store

@neonichu neonichu force-pushed the index-store-based-on-clang branch from 3567d6a to 1ed277c Compare February 5, 2024 20:01
Today, the index store is only enabled on Darwin by default and needs a manual opt-in on other platforms. We can instead switch this to enabling it based on whether the used clang supports `-index-store-path`.

rdar://117744039
@neonichu neonichu force-pushed the index-store-based-on-clang branch from 1ed277c to 913ed0d Compare February 5, 2024 20:01
@neonichu
Copy link
Contributor Author

neonichu commented Feb 5, 2024

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Feb 5, 2024

@swift-ci please test windows

@neonichu neonichu merged commit 5859544 into main Feb 5, 2024
@neonichu neonichu deleted the index-store-based-on-clang branch February 5, 2024 22:39
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.

3 participants