-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[IDE] Avoid printing some Swift extensions twice in mixed source fram… #85501
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
[IDE] Avoid printing some Swift extensions twice in mixed source fram… #85501
Conversation
|
@swift-ci please test |
| continue; | ||
| } | ||
|
|
||
| if (auto extendedTy = Ext->getExtendedNominal()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's already a check in printModuleInterfaceDecl for extensions, which seems like dead code after this change? It currently skips printing extensions if they don't have a clang node and are part of the same module:
if (auto Ext = dyn_cast<ExtensionDecl>(D)) {
// Clang extensions (categories) are always printed in source order.
// Swift extensions are printed with their associated type unless it's
// a cross-module extension.
if (!extensionHasClangNode(Ext)) {
auto ExtendedNominal = Ext->getExtendedNominal();
if (!ExtendedNominal ||
Ext->getModuleContext() == ExtendedNominal->getModuleContext())
return false;
}
}
Though that also means we're skipping more than before. Does this change prevent printing of categories for pure ObjC imports (no Swift)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I forgot to remove this condition which is not no-op and for ObjC categories - they are added to clang decls and would be printed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the old check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this change prevent printing of categories for pure ObjC imports (no Swift)?
Note the check above this means we're only looking at Swift extensions specifically here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the check above this means we're only looking at Swift extensions specifically here
Which check 🤔?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (extensionHasClangNode(Ext)) {
addToClangDecls(Ext, extensionGetClangNode(Ext));
continue;
}62109cf to
7bc7eed
Compare
hamishknight
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
…eworks `printModuleInterfaceDecl` printes extensions right after the type they are associated with is printed. Extensions associated with a type that appears in the "target" module shouldn't be added to `SwiftDecls` because that would lead to double printing them.
7bc7eed to
700d9fd
Compare
|
@swift-ci please test |
…eworks
printModuleInterfaceDeclprintes extensions right after the type they are associated with is printed. Extensions associated with a type that appears in the "target" module shouldn't be added toSwiftDeclsbecause that would lead to double printing them.