Skip to content

[Clang importer] Eliminate redundant imports of C++ fields as properties #66319

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
Jun 3, 2023

Conversation

DougGregor
Copy link
Member

A recent refactoring uncovered two places where we could end up importing a C++ field declaration as a property more than once:

  1. Importing the declaration context of a field in C++ mode can then go import all of the fields. In such a case, check that the field we're importing didn't happen already, and bail out early if it did. This is common practice in the Clang importer but wasn't happening here.
  2. One caller to the function that imported a field from a C++ base class into its inheriting class (as a computed property) wasn't checking the cache, and therefore created a redundant version.

Fix both issues.

A recent refactoring uncovered two places where we could end up
importing a C++ field declaration as a property more than once:

1. Importing the declaration context of a field in C++ mode can then
  go import all of the fields. In such a case, check that the field
  we're importing didn't happen already, and bail out early if it did.
  This is common practice in the Clang importer but wasn't happening here.
2. One caller to the function that imported a field from a C++ base
  class into its inheriting class (as a computed property) wasn't
  checking the cache, and therefore created a redundant version.

Fix both issues.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 4dcd9c5 into swiftlang:main Jun 3, 2023
@DougGregor DougGregor deleted the cxx-import-field-caching branch June 7, 2023 16:52
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