Skip to content

[IRGen] Check as early as possible for Clang decls we've seen before. #31272

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 2 commits into from
Apr 24, 2020

Conversation

martinboehme
Copy link
Contributor

Previously, we were only doing this after the fast-path code that handles decls without any executable code. This meant, however, that we were potentially processing these decls multiple times. This is definitely inefficient; it may even be a correctness issue, depending on what amount of checking HandleTopLevelDecl does to see if it has processed a particular decl before (which I'm not sure about either way).

Previously, we were only doing this after the fast-path code that
handles decls without any executable code. This meant, however, that we
were potentially processing these decls multiple times. This is
definitely inefficient; it may even be a correctness issue, depending on
what amount of checking `HandleTopLevelDecl` does to see if it has
processed a particular decl before (which I'm not sure about either
way).
@gribozavr
Copy link
Contributor

@swift-ci Please test

@gribozavr gribozavr added the c++ interop Feature: Interoperability with C++ label Apr 24, 2020
@gribozavr
Copy link
Contributor

@swift-ci Please test Windows

@gribozavr gribozavr merged commit 2b7849d into swiftlang:master Apr 24, 2020
@martinboehme martinboehme deleted the optimize-emit-clang-decl branch April 27, 2020 06:28
aschwaighofer pushed a commit to aschwaighofer/swift that referenced this pull request Sep 10, 2020
…ang-decl

[IRGen] Check as early as possible for Clang decls we've seen before.
aschwaighofer pushed a commit to aschwaighofer/swift that referenced this pull request Sep 10, 2020
Explanation:
LLVM currently asserts on local extern variables in C headers passed to
Swift when the definition exists outside that header.

Scope: This never worked.

Risk: Low.

Testing: Regression test added.

Reviewed-by: John McCall

Original PR: swiftlang#33306

This fix also required the cherry-pick of: swiftlang#31035 and swiftlang#31272

rdar://67951491
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.

2 participants