Skip to content

[SILGen] Fix the type of closure thunks that are passed const reference structs #76903

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 3 commits into from
Oct 9, 2024

Conversation

ahatanaka
Copy link
Contributor

The thunk's parameter needs the @in_guaranteed convention if it's a
const reference parameter. However, that convention wasn't being used
because clang importer was removing the const reference from the
type and SILGen was computing the type of the parameter based on the
type without const reference.

This commit fixes the bug by passing the clang function type to
SILDeclRef so that it can be used to compute the correct thunk type.

This fixes a crash when a closure is passed to a C function taking a
pointer to a function that has a const reference struct parameter.

This recommits e074426 with fixes to
serialization/deserialization of function types. The fixes prevent clang
types of functions from being dropped during serialization.

rdar://131321096
rdar://136892251

…ce structs (#75491)

The thunk's parameter needs the @in_guaranteed convention if it's a
const reference parameter. However, that convention wasn't being used
because clang importer was removing the const reference from the
type and SILGen was computing the type of the parameter based on the
type without const reference.

This commit fixes the bug by passing the clang function type to
SILDeclRef so that it can be used to compute the correct thunk type.

This fixes a crash when a closure is passed to a C function taking a
pointer to a function that has a const reference struct parameter.

rdar://131321096
@ahatanaka
Copy link
Contributor Author

@swift-ci please test

@ahatanaka
Copy link
Contributor Author

@swift-ci Please Test Source Compatibility Debug

1 similar comment
@ahatanaka
Copy link
Contributor Author

@swift-ci Please Test Source Compatibility Debug

@ahatanaka ahatanaka requested a review from rjmccall October 8, 2024 20:21
@ahatanaka ahatanaka marked this pull request as ready for review October 8, 2024 20:21
@ahatanaka
Copy link
Contributor Author

@swift-ci Please smoke test

1 similar comment
@ahatanaka
Copy link
Contributor Author

@swift-ci Please smoke test

@ahatanaka ahatanaka enabled auto-merge (squash) October 9, 2024 03:17
@ahatanaka ahatanaka merged commit 9c44b79 into main Oct 9, 2024
3 checks passed
@ahatanaka ahatanaka deleted the cxx-const-ref2 branch October 9, 2024 06:44
j-hui pushed a commit to j-hui/swift that referenced this pull request Oct 29, 2024
…ce structs (swiftlang#76903)

The thunk's parameter needs the @in_guaranteed convention if it's a
const reference parameter. However, that convention wasn't being used
because clang importer was removing the const reference from the
type and SILGen was computing the type of the parameter based on the
type without const reference.

This commit fixes the bug by passing the clang function type to
SILDeclRef so that it can be used to compute the correct thunk type.

This fixes a crash when a closure is passed to a C function taking a
pointer to a function that has a const reference struct parameter.

This recommits e074426 with fixes to
serialization/deserialization of function types. The fixes prevent clang
types of functions from being dropped during serialization.

rdar://131321096
ahatanaka added a commit that referenced this pull request Oct 30, 2024
… reference structs (#76903)"

This reverts commit 9c44b79.

The commit caused swift's deserialization code to crash.

rdar://138726860
ahatanaka added a commit that referenced this pull request Oct 31, 2024
… reference structs (#76903)" (#77309)

This reverts commit 9c44b79.

The commit caused swift's deserialization code to crash.

rdar://138726860
Xazax-hun pushed a commit that referenced this pull request Apr 10, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Apr 10, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Apr 22, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Apr 23, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Jun 11, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Jun 11, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Jun 17, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Jun 25, 2025
…ed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.
Xazax-hun pushed a commit that referenced this pull request Jun 25, 2025
…ce structs

This PR is another attempt at landing #76903. The changes compared to
the original PR:
* Instead of increasing the size of SILDeclRef, store the necessary type
  information in the conversion AST nodes.
* The PR above introduced a crash during indexing system modules that
  references foreign types coming from modules imported as
  implementation only. These entities are implementation details so they
  do not need to be included during serialization. This PR adds a test
  and adds logic to exclude such declarations in the serialization
  process.

rdar://131321096&141786724
Xazax-hun pushed a commit that referenced this pull request Jun 27, 2025
…ce structs

This PR is another attempt at landing #76903. The changes compared to
the original PR:
* Instead of increasing the size of SILDeclRef, store the necessary type
  information in the conversion AST nodes.
* The PR above introduced a crash during indexing system modules that
  references foreign types coming from modules imported as
  implementation only. These entities are implementation details so they
  do not need to be included during serialization. This PR adds a test
  and adds logic to exclude such declarations in the serialization
  process.

rdar://131321096&141786724
Xazax-hun pushed a commit that referenced this pull request Jun 27, 2025
…ce structs

This PR is another attempt at landing #76903. The changes compared to
the original PR:
* Instead of increasing the size of SILDeclRef, store the necessary type
  information in the conversion AST nodes.
* The PR above introduced a crash during indexing system modules that
  references foreign types coming from modules imported as
  implementation only. These entities are implementation details so they
  do not need to be included during serialization. This PR adds a test
  and adds logic to exclude such declarations in the serialization
  process.

rdar://131321096&141786724
Xazax-hun pushed a commit that referenced this pull request Jun 27, 2025
…ce structs

This PR is another attempt at landing #76903. The changes compared to
the original PR:
* Instead of increasing the size of SILDeclRef, store the necessary type
  information in the conversion AST nodes.
* The PR above introduced a crash during indexing system modules that
  references foreign types coming from modules imported as
  implementation only. These entities are implementation details so they
  do not need to be included during serialization. This PR adds a test
  and adds logic to exclude such clang types in the serialization
  process.

rdar://131321096&141786724
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.

1 participant