Skip to content

[cxx-interop] Remove some duplicated lookups #79324

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 27, 2025
Merged

Conversation

Xazax-hun
Copy link
Contributor

Repeatedly lookup up a key from a dictionary can be justified whenever the content of the dictionary might change between the lookups (so any references into the dictionary might get invalidated). We had a couple of instances where as far as I can tell no such modifications should happen between two lookups with identical keys. This PR simplifies the code to remove the extra lookups. It also removes a dictionary that was completely unused.

@Xazax-hun Xazax-hun added the c++ interop Feature: Interoperability with C++ label Feb 12, 2025
@Xazax-hun Xazax-hun requested a review from j-hui February 12, 2025 16:08
auto known = ImportedMacros.find(name);
if (known == ImportedMacros.end()) {
auto [known, inserted] = ImportedMacros.try_emplace(
name, SmallVector<std::pair<const clang::MacroInfo *, ValueDecl *>, 2>{});
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use something like auto here to avoid repeating the type name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, I could not avoid naming the type here because the value for try_emplace is taking a template so there is no context to deduce the type of this SmallVector from.

Repeatedly lookup up a key from a dictionary can be justified whenever
the content of the dictionary might change between the lookups (so any
references into the dictionary might get invalidated). We had a couple
of instances where as far as I can tell no such modifications should
happen between two lookups with identical keys. This PR simplifies the
code to remove the extra lookups. It also removes a dictionary that was
completely unused.
@Xazax-hun Xazax-hun force-pushed the gaborh/redundant-lookups branch from da5e498 to 068815c Compare February 14, 2025 11:04
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@j-hui j-hui left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Xazax-hun Xazax-hun merged commit ed9eef2 into main Feb 27, 2025
3 checks passed
@Xazax-hun Xazax-hun deleted the gaborh/redundant-lookups branch February 27, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants