Skip to content

[c++-interop] Providing information about enum types from inferDefaultArgument #59421

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 17, 2022

Conversation

plotfi
Copy link
Contributor

@plotfi plotfi commented Jun 13, 2022

When ClangImporter::Implementation::inferDefaultArgument processes
func/method arguments as part of omitNeedlessWordsInFunctionName it
processes information about how the typenames for the parameters related
to the parameter names to form a parameter names list. The parameter
names list is used to determine if the argument label for a function
should be clipped based on the typename. So for example a type like
NSOrderedCollectionDifferenceCalculationOptions would cause a label
ending with "Options" to get clipped so that for instance "withOptions"
becomes simply "with".

Unfortunately in the context of C++-Interop, the typename for the
parameter often resolves to what the type backing the typedef or enum is
and not the actual name of the typedef
(so typedef NSUInteger NSOrderedCollectionDifferenceCalculationOptions
resolves to a name of NSUInteger rather than
NSOrderedCollectionDifferenceCalculationOptions).

This patch seeks to collect a bit more information when processing
NS_OPTIONS typedefs and providing that to the calling
omitNeedlessWordsInFunctionName to handle more inteligently.

In practice this fixes anywhere in Foundatio where

withOptions: NSOrderedCollectionDifferenceCalculationOptions is used.

NOTE: This is a work in progress. Still trying to figure out how to reduce a good test case.

@plotfi plotfi requested review from zoecarver and hyp June 13, 2022 23:33
@plotfi plotfi added the c++ interop Feature: Interoperability with C++ label Jun 13, 2022
Copy link
Contributor

@zoecarver zoecarver left a comment

Choose a reason for hiding this comment

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

LGTM with some tests

@plotfi plotfi force-pushed the typedef-ns-options-name-fixup branch from 5749dd4 to a0084e3 Compare June 16, 2022 07:17
@plotfi
Copy link
Contributor Author

plotfi commented Jun 16, 2022

With my latest changes almost all of the omitNeedlessWords differences presented in https://docs.google.com/spreadsheets/d/1FswVLDXhYPqufMsVfxhF5Q5l5PW19J_JcZq67Cai6KQ/edit#gid=0

are fixed:

https://github.dev/plotfi/cxx-interop-diff/pull/6

@plotfi plotfi force-pushed the typedef-ns-options-name-fixup branch 2 times, most recently from 6f93671 to 0fe159d Compare June 16, 2022 21:41
…tArgument

When ClangImporter::Implementation::inferDefaultArgument processes
func/method arguments as part of omitNeedlessWordsInFunctionName it
processes information about how the typenames for the parameters related
to the parameter names to form a parameter names list. The parameter
names list is used to determine if the argument label for a function
should be clipped based on the typename. So for example a type like
NSOrderedCollectionDifferenceCalculationOptions would cause a label
ending with "Options" to get clipped so that for instance "withOptions"
becomes simply "with".

Unfortunately in the context of C++-Interop, the typename for the
parameter often resolves to what the type backing the typedef or enum is
and not the actual name of the typedef
(so `typedef NSUInteger NSOrderedCollectionDifferenceCalculationOptions`
 resolves to a name of NSUInteger rather than
 NSOrderedCollectionDifferenceCalculationOptions).

This patch seeks to collect a bit more information when processing
NS_OPTIONS typedefs and providing that to the calling
omitNeedlessWordsInFunctionName to handle more inteligently.

In practice this fixes anywhere in Foundatio where

`withOptions: NSOrderedCollectionDifferenceCalculationOptions` is used.
@plotfi plotfi force-pushed the typedef-ns-options-name-fixup branch from 0fe159d to 28375ae Compare June 17, 2022 02:41
@plotfi
Copy link
Contributor Author

plotfi commented Jun 17, 2022

LGTM with some tests

Test cases added.

@bulbazord
Copy link
Contributor

@swift-ci please test

@plotfi plotfi merged commit 8a546a9 into swiftlang:main Jun 17, 2022
NuriAmari added a commit to NuriAmari/swift that referenced this pull request Feb 16, 2023
When Objective-c methods are imported into Swift, the labels are
transforms to remove suffixes based on the type of the parameter. Due
to the structure of NS_OPTION when importing in C++ mode, there is
some special handling for this case introduced in PR swiftlang#59421.

This patch fixes some of this hanlding for when a multi-word suffix
needs removing, specifically 'ControlEvents' and 'ScrollPosition'.
NuriAmari added a commit to NuriAmari/swift that referenced this pull request Feb 16, 2023
When Objective-c methods are imported into Swift, the labels are
transforms to remove suffixes based on the type of the parameter. Due
to the structure of NS_OPTION when importing in C++ mode, there is
some special handling for this case introduced in PR swiftlang#59421.

This patch fixes some of this hanlding for when a multi-word suffix
needs removing, specifically 'ControlEvents' and 'ScrollPosition'.
NuriAmari added a commit to NuriAmari/swift that referenced this pull request Feb 16, 2023
When Objective-c methods are imported into Swift, the labels are
transforms to remove suffixes based on the type of the parameter. Due
to the structure of NS_OPTION when importing in C++ mode, there is
some special handling for this case introduced in PR swiftlang#59421.

This patch fixes some of this hanlding for when a multi-word suffix
needs removing, specifically 'ControlEvents' and 'ScrollPosition'.
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