Skip to content

Commit f27005b

Browse files
committed
ModuleInterface: When printing synthesized extensions, we need to be sure to guard them with required features if applicable. Not doing so can result in broken interfaces that do not typecheck because, for instance, a conformance can refer to a nominal type that is only declared when certain features are enabled.
Also, fix a typo where `#elsif` was printed into interfaces instead of `#elseif`. Resolves rdar://91509673
1 parent 728f77c commit f27005b

File tree

4 files changed

+82
-63
lines changed

4 files changed

+82
-63
lines changed

include/swift/AST/PrintOptions.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,12 @@ struct PrintOptions {
391391
/// Whether to use an empty line to separate two members in a single decl.
392392
bool EmptyLineBetweenMembers = false;
393393

394+
/// Whether to print empty members of a declaration on a single line, e.g.:
395+
/// ```
396+
/// extension Foo: Bar {}
397+
/// ```
398+
bool PrintEmptyMembersOnSameLine = false;
399+
394400
/// Whether to print the extensions from conforming protocols.
395401
bool PrintExtensionFromConformingProtocols = false;
396402

lib/AST/ASTPrinter.cpp

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2335,7 +2335,8 @@ void PrintAST::printMembers(ArrayRef<Decl *> members, bool needComma,
23352335
bool openBracket, bool closeBracket) {
23362336
if (openBracket) {
23372337
Printer << " {";
2338-
Printer.printNewline();
2338+
if (!Options.PrintEmptyMembersOnSameLine || !members.empty())
2339+
Printer.printNewline();
23392340
}
23402341
{
23412342
IndentRAII indentMore(*this);
@@ -3128,13 +3129,12 @@ static FeatureSet getUniqueFeaturesUsed(Decl *decl) {
31283129
return features;
31293130
}
31303131

3131-
static void printCompatibilityCheckIf(ASTPrinter &printer,
3132-
bool isElsif,
3132+
static void printCompatibilityCheckIf(ASTPrinter &printer, bool isElseIf,
31333133
bool includeCompilerCheck,
31343134
const BasicFeatureSet &features) {
31353135
assert(!features.empty());
31363136

3137-
printer << (isElsif ? "#elsif " : "#if ");
3137+
printer << (isElseIf ? "#elseif " : "#if ");
31383138
if (includeCompilerCheck)
31393139
printer << "compiler(>=5.3) && ";
31403140

@@ -3150,7 +3150,7 @@ static void printCompatibilityCheckIf(ASTPrinter &printer,
31503150
printer.printNewline();
31513151
}
31523152

3153-
/// Generate a #if ... #elsif ... #endif chain for the given
3153+
/// Generate a #if ... #elseif ... #endif chain for the given
31543154
/// suppressible feature checks.
31553155
static void printWithSuppressibleFeatureChecks(ASTPrinter &printer,
31563156
PrintOptions &options,
@@ -3171,18 +3171,17 @@ static void printWithSuppressibleFeatureChecks(ASTPrinter &printer,
31713171
return;
31723172
}
31733173

3174-
// Otherwise, enter a `#if` or `#elsif` for the next feature.
3174+
// Otherwise, enter a `#if` or `#elseif` for the next feature.
31753175
Feature feature = generator.next();
3176-
printCompatibilityCheckIf(printer, /*elsif*/ !firstInChain,
3177-
includeCompilerCheck,
3178-
{feature});
3176+
printCompatibilityCheckIf(printer, /*elseif*/ !firstInChain,
3177+
includeCompilerCheck, {feature});
31793178

31803179
// Print the body.
31813180
printBody();
31823181
printer.printNewline();
31833182

31843183
// Start suppressing the feature and recurse to either generate
3185-
// more `#elsif` clauses or finish off with `#endif`.
3184+
// more `#elseif` clauses or finish off with `#endif`.
31863185
suppressingFeature(options, feature, [&] {
31873186
printWithSuppressibleFeatureChecks(printer, options, /*first*/ false,
31883187
includeCompilerCheck, generator,
@@ -3195,13 +3194,13 @@ static void printWithSuppressibleFeatureChecks(ASTPrinter &printer,
31953194
///
31963195
/// In the most general form, with both required features and multiple
31973196
/// suppressible features in play, the generated code pattern looks like
3198-
/// the following (assuming that feaature $bar implies feature $baz):
3197+
/// the following (assuming that feature $bar implies feature $baz):
31993198
///
32003199
/// ```
32013200
/// #if compiler(>=5.3) && $foo
32023201
/// #if $bar
32033202
/// @foo @bar @baz func @test() {}
3204-
/// #elsif $baz
3203+
/// #elseif $baz
32053204
/// @foo @baz func @test() {}
32063205
/// #else
32073206
/// @foo func @test() {}
@@ -3229,7 +3228,7 @@ void swift::printWithCompatibilityFeatureChecks(ASTPrinter &printer,
32293228
bool hasRequiredFeatures = features.hasAnyRequired();
32303229
if (hasRequiredFeatures) {
32313230
printCompatibilityCheckIf(printer,
3232-
/*elsif*/ false,
3231+
/*elseif*/ false,
32333232
/*compiler check*/ true,
32343233
features.requiredFeatures());
32353234
}

lib/Frontend/ModuleInterfaceSupport.cpp

Lines changed: 56 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
#include "swift/AST/FileSystem.h"
2020
#include "swift/AST/Module.h"
2121
#include "swift/AST/ModuleNameLookup.h"
22+
#include "swift/AST/NameLookupRequests.h"
2223
#include "swift/AST/ProtocolConformance.h"
24+
#include "swift/AST/TypeCheckRequests.h"
2325
#include "swift/AST/TypeRepr.h"
2426
#include "swift/Basic/STLExtras.h"
2527
#include "swift/Frontend/Frontend.h"
@@ -569,13 +571,14 @@ class InheritedProtocolCollector {
569571
});
570572
}
571573

574+
// Preserve the behavior of previous implementations which formatted of
575+
// empty extensions compactly with '{}' on the same line.
576+
PrintOptions extensionPrintOptions = printOptions;
577+
extensionPrintOptions.PrintEmptyMembersOnSameLine = true;
578+
572579
// Then walk the remaining ones, and see what we need to print.
573-
// Note: We could do this in one pass, but the logic is easier to
574-
// understand if we build up the list and then print it, even if it takes
575-
// a bit more memory.
576580
// FIXME: This will pick the availability attributes from the first sight
577581
// of a protocol rather than the maximally available case.
578-
SmallVector<ProtocolAndAvailability, 16> protocolsToPrint;
579582
for (const auto &protoAndAvailability : ExtraProtocols) {
580583
auto proto = std::get<0>(protoAndAvailability);
581584
auto availability = std::get<1>(protoAndAvailability);
@@ -601,58 +604,64 @@ class InheritedProtocolCollector {
601604
if (isPublicOrUsableFromInline(inherited) &&
602605
conformanceDeclaredInModule(M, nominal, inherited) &&
603606
!M->isImportedImplementationOnly(inherited->getParentModule())) {
604-
protocolsToPrint.push_back(
605-
ProtocolAndAvailability(inherited, availability, isUnchecked,
606-
otherAttrs));
607+
auto protoAndAvailability = ProtocolAndAvailability(
608+
inherited, availability, isUnchecked, otherAttrs);
609+
printSynthesizedExtension(out, extensionPrintOptions, M, nominal,
610+
protoAndAvailability);
607611
return TypeWalker::Action::SkipChildren;
608612
}
609613

610614
return TypeWalker::Action::Continue;
611615
});
612616
}
613-
if (protocolsToPrint.empty())
614-
return;
615-
616-
for (const auto &protoAndAvailability : protocolsToPrint) {
617-
StreamPrinter printer(out);
618-
auto proto = std::get<0>(protoAndAvailability);
619-
auto availability = std::get<1>(protoAndAvailability);
620-
auto isUnchecked = std::get<2>(protoAndAvailability);
621-
auto otherAttrs = std::get<3>(protoAndAvailability);
622-
623-
PrintOptions curPrintOptions = printOptions;
624-
auto printBody = [&] {
625-
// FIXME: Shouldn't this be an implicit conversion?
626-
TinyPtrVector<const DeclAttribute *> attrs;
627-
attrs.insert(attrs.end(), availability.begin(), availability.end());
628-
auto spiAttributes = proto->getAttrs().getAttributes<SPIAccessControlAttr>();
629-
attrs.insert(attrs.end(), spiAttributes.begin(), spiAttributes.end());
630-
attrs.insert(attrs.end(), otherAttrs.begin(), otherAttrs.end());
631-
DeclAttributes::print(printer, curPrintOptions, attrs);
632-
633-
printer << "extension ";
634-
{
635-
bool oldFullyQualifiedTypesIfAmbiguous =
636-
curPrintOptions.FullyQualifiedTypesIfAmbiguous;
637-
curPrintOptions.FullyQualifiedTypesIfAmbiguous =
638-
curPrintOptions.FullyQualifiedExtendedTypesIfAmbiguous;
639-
nominal->getDeclaredType().print(printer, curPrintOptions);
640-
curPrintOptions.FullyQualifiedTypesIfAmbiguous =
641-
oldFullyQualifiedTypesIfAmbiguous;
642-
}
643-
printer << " : ";
644-
645-
if (isUnchecked)
646-
printer << "@unchecked ";
617+
}
647618

648-
proto->getDeclaredInterfaceType()->print(printer, curPrintOptions);
619+
/// Prints a dummy extension on \p nominal to \p out for a public conformance
620+
/// to the protocol contained by \p protoAndAvailability.
621+
static void
622+
printSynthesizedExtension(raw_ostream &out, const PrintOptions &printOptions,
623+
ModuleDecl *M, const NominalTypeDecl *nominal,
624+
ProtocolAndAvailability &protoAndAvailability) {
625+
StreamPrinter printer(out);
626+
627+
auto proto = std::get<0>(protoAndAvailability);
628+
auto availability = std::get<1>(protoAndAvailability);
629+
auto isUnchecked = std::get<2>(protoAndAvailability);
630+
auto otherAttrs = std::get<3>(protoAndAvailability);
631+
632+
// Create a synthesized ExtensionDecl for the conformance.
633+
ASTContext &ctx = M->getASTContext();
634+
auto inherits = ctx.AllocateCopy(llvm::makeArrayRef(InheritedEntry(
635+
TypeLoc::withoutLoc(proto->getDeclaredInterfaceType()), isUnchecked)));
636+
auto extension =
637+
ExtensionDecl::create(ctx, SourceLoc(), nullptr, inherits,
638+
nominal->getModuleScopeContext(), nullptr);
639+
extension->setImplicit();
640+
641+
// Build up synthesized DeclAttributes for the extension.
642+
TinyPtrVector<const DeclAttribute *> attrs;
643+
attrs.insert(attrs.end(), availability.begin(), availability.end());
644+
auto spiAttributes =
645+
proto->getAttrs().getAttributes<SPIAccessControlAttr>();
646+
attrs.insert(attrs.end(), spiAttributes.begin(), spiAttributes.end());
647+
attrs.insert(attrs.end(), otherAttrs.begin(), otherAttrs.end());
648+
649+
// Since DeclAttributes is a linked list where each added attribute becomes
650+
// the head, we need to add these attributes in reverse order to reproduce
651+
// the order in which previous implementations printed these attributes.
652+
DeclAttributes declAttrs;
653+
for (auto attr = attrs.rbegin(), end = attrs.rend(); attr != end; ++attr) {
654+
declAttrs.add(const_cast<DeclAttribute *>(*attr));
655+
}
656+
extension->getAttrs() = declAttrs;
649657

650-
printer << " {}";
651-
};
658+
ctx.evaluator.cacheOutput(ExtendedTypeRequest{extension},
659+
nominal->getDeclaredType());
660+
ctx.evaluator.cacheOutput(ExtendedNominalRequest{extension},
661+
const_cast<NominalTypeDecl *>(nominal));
652662

653-
printBody();
654-
printer << "\n";
655-
}
663+
extension->print(printer, printOptions);
664+
printer << "\n";
656665
}
657666

658667
/// If there were any conditional conformances that couldn't be printed,

test/ModuleInterface/features.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
// RUN: %empty-directory(%t)
22

3-
// RUN: %target-swift-frontend -typecheck -swift-version 5 -module-name FeatureTest -emit-module-interface-path - -enable-library-evolution -disable-availability-checking %s | %FileCheck %s
4-
// REQUIRES: concurrency
3+
// RUN: %target-swift-frontend -typecheck -swift-version 5 -module-name FeatureTest -emit-module-interface-path %t/FeatureTest.swiftinterface -enable-library-evolution -disable-availability-checking %s
4+
// RUN: %FileCheck %s < %t/FeatureTest.swiftinterface
5+
// RUN: %target-swift-frontend -typecheck-module-from-interface -disable-availability-checking -swift-version 5 -module-name FeatureTest %t/FeatureTest.swiftinterface
56

67
// REQUIRES: concurrency
78

@@ -166,7 +167,7 @@ public func unsafeInheritExecutor() async {}
166167
// CHECK-NEXT: #if $UnsafeInheritExecutor
167168
// CHECK-NEXT: @_specialize{{.*}}
168169
// CHECK-NEXT: @_unsafeInheritExecutor public func multipleSuppressible<T>(value: T) async
169-
// CHECK-NEXT: #elsif $SpecializeAttributeWithAvailability
170+
// CHECK-NEXT: #elseif $SpecializeAttributeWithAvailability
170171
// CHECK-NEXT: @_specialize{{.*}}
171172
// CHECK-NEXT: public func multipleSuppressible<T>(value: T) async
172173
// CHECK-NEXT: #else
@@ -195,3 +196,7 @@ public func unavailableFromAsyncFunc() { }
195196
public func noAsyncFunc() { }
196197

197198
// CHECK-NOT: extension FeatureTest.MyActor : Swift.Sendable
199+
200+
// CHECK: #if compiler(>=5.3) && $GlobalActors
201+
// CHECK-NEXT: extension FeatureTest.SomeGlobalActor : _Concurrency.GlobalActor {}
202+
// CHECK-NEXT: #endif

0 commit comments

Comments
 (0)