Skip to content

Move OptionSet to its own namespace #69785

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
Nov 14, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Nov 10, 2023

The type was colliding with the OptionSet type in the Swift stdlib when using Swift from C++ (ie. when we were importing a the generated <SwiftModule>-Swift.h bridging header.

@ahoppen
Copy link
Member Author

ahoppen commented Nov 10, 2023

@swift-ci Please smoke test

@@ -151,7 +152,7 @@ class OptionSet {
Flags>::value,
"operator| should produce an OptionSet");
};

} // end namespace optionset
Copy link
Member

@rintaro rintaro Nov 13, 2023

Choose a reason for hiding this comment

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

Does using namespace have any effect in Swift? If not, how about

Suggested change
} // end namespace optionset
} // end namespace optionset
using namespace optionset;

and that's it (with some doc-comment for this weird hack).

Copy link
Contributor

Choose a reason for hiding this comment

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

using namespace is ignored by Swift when it imports C++ headers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, nice idea. I hadn’t thought of that. I tried using OptionSet = optionset::OptionSet, which didn’t work but this is much cleaner (and doesn’t require any downstream changes).

@xedin xedin removed their request for review November 13, 2023 19:11
The type was colliding with the `OptionSet` type in the Swift stdlib when using Swift from C++.
@ahoppen ahoppen force-pushed the ahoppen/move-optionset branch from 9224f14 to 62ffdf2 Compare November 13, 2023 22:26
@ahoppen
Copy link
Member Author

ahoppen commented Nov 13, 2023

@swift-ci Please smoke test

@ahoppen ahoppen merged commit fdca46c into swiftlang:main Nov 14, 2023
@ahoppen ahoppen deleted the ahoppen/move-optionset branch November 14, 2023 17:55
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