Skip to content

Serialization: Ignore lookup shadowing when resolving cross-references #80009

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
Mar 18, 2025

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Mar 14, 2025

When a Swift function shadows a clang function of the same name, the assumption was that Swift code would refer only to the Swift one. However, if the Swift function is @usableFromInline internal it can be called only from the local module and inlined automatically in other clients. Outside of that module, sources see only the clang function, so their inlinable code calls only the clang function and ignores the Swift one. This configuration passed type checking but it could crash the compiler at inlining the call as the compiler couldn't see the clang function.

Let's update the deserialization logic to support inlined calls to the shadowed or the shadower. Typical shadowing is already handled by the custom deserialization cross-reference filtering logic which looks for the defining module, scope and whether it's a Swift or clang decl. We can disable the lookup shadowing logic and rely only on the deserialization filtering.

rdar://146320871
#79801

@xymus
Copy link
Contributor Author

xymus commented Mar 14, 2025

@swift-ci Please smoke test

@xymus
Copy link
Contributor Author

xymus commented Mar 14, 2025

@swift-ci please test source compatibility

xymus added 2 commits March 14, 2025 12:35
When a Swift function shadows a clang function of the same name, the
assumption was that Swift code would refer only to the Swift one.
However, if the Swift function is `@usableFromInline internal` it can be
called only from the local module and inlined automatically in other
clients. Outside of that module, sources see only the clang function, so
their inlinable code calls only the clang function and ignores the Swift
one. This configuration passed type checking but it could crash the
compiler at inlining the call as the compiler couldn't see the clang
function.

Let's update the deserialization logic to support inlined calls to the
shadowed or the shadower. Typical shadowing is already handled by the
custom deserialization cross-reference filtering logic which looks for
the defining module, scope and whether it's a Swift or clang decl. We
can disable the lookup shadowing logic and rely only on the
deserialization filtering.

rdar://146320871
swiftlang#79801
@xymus xymus force-pushed the deser-internal-shadow branch from 38f3f34 to bf224e2 Compare March 14, 2025 20:36
@xymus
Copy link
Contributor Author

xymus commented Mar 14, 2025

Adding a test covering more shadowing scenarios.

@swift-ci Please smoke test

@xymus xymus requested review from artemcm and tshortli March 18, 2025 16:48
@xymus xymus merged commit 7ae7371 into swiftlang:main Mar 18, 2025
3 checks passed
@xymus xymus deleted the deser-internal-shadow branch March 18, 2025 17:42
xedin added a commit to xedin/swift-source-compat-suite that referenced this pull request Mar 22, 2025
justice-adams-apple pushed a commit to swiftlang/swift-source-compat-suite that referenced this pull request Mar 24, 2025
xymus added a commit to xymus/swift that referenced this pull request Apr 4, 2025
Followup fix to swiftlang#80009. We can still get ambiguities from colliding
decls across modules with the deserialization filtering. Bring back
calling the general lookup shadowing after the filtering. This way it
won't use filtered out decls to hide potential candidates.

rdar://148286345
xymus added a commit to xymus/swift that referenced this pull request Apr 8, 2025
Followup fix to swiftlang#80009. We can still get ambiguities from colliding
decls across modules with the deserialization filtering. Bring back
calling the general lookup shadowing after the filtering. This way it
won't use filtered out decls to hide potential candidates.

rdar://148286345
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