Skip to content

Create feature flag for SE-0384 #64730

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
Mar 29, 2023

Conversation

NuriAmari
Copy link
Contributor

No description provided.

@NuriAmari
Copy link
Contributor Author

@swift-ci Please test

@NuriAmari NuriAmari requested a review from allevato March 29, 2023 17:10
@NuriAmari NuriAmari marked this pull request as ready for review March 29, 2023 17:11
@NuriAmari NuriAmari force-pushed the fwd-declarations-feature-flag branch from 7cc7f2b to 564621e Compare March 29, 2023 17:27
@NuriAmari
Copy link
Contributor Author

@swift-ci Please test

@@ -3159,6 +3159,24 @@ static bool usesFeatureExistentialAny(Decl *decl) {
return false;
}

static bool usesFeatureImportObjcForwardDeclarations(Decl *decl) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this being used for anything? I don't see it referenced elsewhere in this PR.

Copy link
Contributor Author

@NuriAmari NuriAmari Mar 29, 2023

Choose a reason for hiding this comment

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

Yes, but not obviously -- above in the same file:

#define LANGUAGE_FEATURE(FeatureName, SENumber, Description, Option)  \
    if (usesFeature##FeatureName(decl))                               \
      collectRequiredFeature(Feature::FeatureName, operation);
#define SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description, Option)  \
    if (usesFeature##FeatureName(decl))                               \
      collectSuppressibleFeature(Feature::FeatureName, operation);
#include "swift/Basic/Features.def"
  }
};

It won't compile without this.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, metaprogramming. Thanks for clarifying!

Copy link
Member

@allevato allevato left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround on this! Once this is merged, can you please create a cherry-pick into the 5.9 branch, and also update the text of SE-0384 to refer to the feature name (and remove the reference to the old flag)?

@@ -3159,6 +3159,24 @@ static bool usesFeatureExistentialAny(Decl *decl) {
return false;
}

static bool usesFeatureImportObjcForwardDeclarations(Decl *decl) {
Copy link
Member

Choose a reason for hiding this comment

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

Ah, metaprogramming. Thanks for clarifying!

@NuriAmari NuriAmari merged commit e219699 into swiftlang:main Mar 29, 2023
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.

2 participants