[InterfaceGen] Print abstract accessors in protocols (#19379)

Harlan · web-flow · commit 8ba8222333d0 · 2018-09-20T15:43:01.000-07:00
* [InterfaceGen] Print abstract accessors in protocols

This patch slightly cleans up printing accessors and ensures we print
accessors abstractly in protocol context for textual interfaces.

It also removes some assuptions around the FunctionBody callback and
makes them more explicit.

* Print getter and setter for didSet decls

* Test _read and _modify

* Fix logic for skipping willSet/didSet

* Update 'final' test for new getter printing behavior
diff --git a/include/swift/AST/PrintOptions.h b/include/swift/AST/PrintOptions.h
@@ -135,13 +135,32 @@ struct PrintOptions {
   /// \brief Whether to print *any* accessors on properties.
   bool PrintPropertyAccessors = true;
 
-  /// \brief Whether to print the accessors of a property abstractly,
-  /// i.e. always as get and set rather than the specific accessors
-  /// actually used to implement the property.
+  /// Whether to print the accessors of a property abstractly,
+  /// i.e. always as:
+  /// ```
+  /// var x: Int { get set }
+  /// ```
+  /// rather than the specific accessors actually used to implement the
+  /// property.
   ///
   /// Printing function definitions takes priority over this setting.
   bool AbstractAccessors = true;
 
+  /// Whether to print a property with only a single getter using the shorthand
+  /// ```
+  /// var x: Int { return y }
+  /// ```
+  /// vs.
+  /// ```
+  /// var x: Int {
+  ///   get { return y }
+  /// }
+  /// ```
+  bool CollapseSingleGetterProperty = true;
+
+  /// Whether to print the bodies of accessors in protocol context.
+  bool PrintAccessorBodiesInProtocols = false;
+
   /// \brief Whether to print type definitions.
   bool TypeDefinitions = false;
 
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
@@ -80,6 +80,8 @@ PrintOptions PrintOptions::printTextualInterfaceFile() {
   result.FullyQualifiedTypes = true;
   result.SkipImports = true;
   result.OmitNameOfInaccessibleProperties = true;
+  result.FunctionDefinitions = true;
+  result.CollapseSingleGetterProperty = false;
 
   result.FunctionBody = [](const ValueDecl *decl, ASTPrinter &printer) {
     auto AFD = dyn_cast<AbstractFunctionDecl>(decl);
@@ -1579,15 +1581,15 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
     return;
   }
 
-  // AbstractAccessors is suppressed by FunctionDefinitions, but not the
-  // presence of an FunctionBody callback.
+  // AbstractAccessors is suppressed by FunctionDefinitions.
   bool PrintAbstract =
     Options.AbstractAccessors && !Options.FunctionDefinitions;
 
   // We sometimes want to print the accessors abstractly
   // instead of listing out how they're actually implemented.
   bool inProtocol = isa<ProtocolDecl>(ASD->getDeclContext());
-  if (!Options.FunctionBody && (inProtocol || PrintAbstract)) {
+  if ((inProtocol && !Options.PrintAccessorBodiesInProtocols) ||
+      PrintAbstract) {
     bool mutatingGetter = ASD->getGetter() && ASD->isGetterMutating();
     bool settable = ASD->isSettable(nullptr);
     bool nonmutatingSetter = false;
@@ -1650,12 +1652,13 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
   }
 
   // Otherwise, print all the concrete defining accessors.
+  bool PrintAccessorBody = Options.FunctionDefinitions;
 
-  bool PrintAccessorBody = Options.FunctionDefinitions || Options.FunctionBody;
-
-  auto PrintAccessor = [&](AccessorDecl *Accessor) {
-    if (!Accessor)
-      return;
+  // Helper to print an accessor. Returns true if the
+  // accessor was present but skipped.
+  auto PrintAccessor = [&](AccessorDecl *Accessor) -> bool {
+    if (!Accessor || !shouldPrint(Accessor))
+      return true;
     if (!PrintAccessorBody) {
       if (isAccessorAssumedNonMutating(Accessor->getAccessorKind())) {
         if (Accessor->isMutating()) {
@@ -1679,19 +1682,17 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
       indent();
       Printer.printNewline();
     }
+    return false;
   };
 
-  if ((PrintAbstract ||
-       (impl.getReadImpl() == ReadImplKind::Get && ASD->getGetter())) &&
-      !ASD->supportsMutation() && !ASD->isGetterMutating() &&
-      PrintAccessorBody && !Options.FunctionDefinitions) {
-    // Omit the 'get' keyword. Directly print getter
-    if (auto BodyFunc = Options.FunctionBody) {
-      BodyFunc(ASD->getGetter(), Printer);
-      indent();
-      return;
-    }
-    Printer.printNewline();
+  // Determine if we should print the getter without the 'get { ... }'
+  // block around it.
+  bool isOnlyGetter = impl.getReadImpl() == ReadImplKind::Get &&
+                      ASD->getGetter();
+  bool isGetterMutating = ASD->supportsMutation() || ASD->isGetterMutating();
+  if (isOnlyGetter && !isGetterMutating && PrintAccessorBody &&
+      Options.FunctionBody && Options.CollapseSingleGetterProperty) {
+    Options.FunctionBody(ASD->getGetter(), Printer);
     indent();
     return;
   }
@@ -1726,10 +1727,15 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
     case WriteImplKind::Stored:
       llvm_unreachable("simply-stored variable should have been filtered out");
     case WriteImplKind::StoredWithObservers:
-    case WriteImplKind::InheritedWithObservers:
-      PrintAccessor(ASD->getWillSetFunc());
-      PrintAccessor(ASD->getDidSetFunc());
+    case WriteImplKind::InheritedWithObservers: {
+      bool skippedWillSet = PrintAccessor(ASD->getWillSetFunc());
+      bool skippedDidSet = PrintAccessor(ASD->getDidSetFunc());
+      if (skippedDidSet && skippedWillSet) {
+        PrintAccessor(ASD->getGetter());
+        PrintAccessor(ASD->getSetter());
+      }
       break;
+    }
     case WriteImplKind::Set:
       PrintAccessor(ASD->getSetter());
       if (!shouldHideModifyAccessor())
@@ -1751,9 +1757,6 @@ void PrintAST::printAccessors(AbstractStorageDecl *ASD) {
 
   Printer << "}";
 
-  if (!PrintAbstract)
-    Printer.printNewline();
-
   indent();
 }
 
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -2397,6 +2397,8 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
     Options.AccessFilter = AccessLevel::Private;
     Options.PrintAccess = false;
     Options.SkipAttributes = true;
+    Options.FunctionDefinitions = true;
+    Options.PrintAccessorBodiesInProtocols = true;
     Options.FunctionBody = [&](const ValueDecl *VD, ASTPrinter &Printer) {
       Printer << " {";
       Printer.printNewline();
diff --git a/test/ModuleInterface/final.swift b/test/ModuleInterface/final.swift
@@ -6,8 +6,10 @@
 // CHECK: final public class FinalClass {
 public final class FinalClass {
   // CHECK: @inlinable final public class var a: [[INT:(Swift.)?Int]] {
+  // CHECK-NEXT: {{^}} get {
   // CHECK-NEXT: return 3
   // CHECK-NEXT: }
+  // CHECK-NEXT: }
   @inlinable
   public final class var a: Int {
     return 3
diff --git a/test/ModuleInterface/inlinable-function.swift b/test/ModuleInterface/inlinable-function.swift
@@ -37,8 +37,10 @@ public struct Foo: Hashable {
 
 
   // CHECK: @_transparent public var transparent: [[INT]] {
+  // CHECK-NEXT:   get {
   // CHECK-NEXT:   return 34
   // CHECK-NEXT: }
+  // CHECK-NEXT: }
   @_transparent
   public var transparent: Int {
     return 34
@@ -102,6 +104,78 @@ public struct Foo: Hashable {
     // CHECK-NEXT: }
   }
 
+  // CHECK: @inlinable public var inlinableReadAndModify: [[INT]] {
+  @inlinable
+  public var inlinableReadAndModify: Int {
+    // CHECK: _read {
+    // CHECK-NEXT: yield 0
+    // CHECK-NEXT: }
+    _read {
+      yield 0
+    }
+    // CHECK: _modify {
+    // CHECK-NEXT: var x = 0
+    // CHECK-NEXT: yield &x
+    // CHECK-NEXT: }
+    _modify {
+      var x = 0
+      yield &x
+    }
+    // CHECK-NEXT: }
+  }
+
+  // CHECK: public var inlinableReadNormalModify: [[INT]] {
+  public var inlinableReadNormalModify: Int {
+    // CHECK: @inlinable _read {
+    // CHECK-NEXT: yield 0
+    // CHECK-NEXT: }
+    @inlinable _read {
+      yield 0
+    }
+
+    // CHECK: _modify{{$}}
+    // CHECK-NOT: var x = 0
+    // CHECK-NOT: yield &x
+    // CHECK-NOT: }
+    _modify {
+      var x = 0
+      yield &x
+    }
+    // CHECK-NEXT: }
+  }
+
+  // CHECK: public var normalReadInlinableModify: [[INT]] {
+  public var normalReadInlinableModify: Int {
+    // CHECK: _read{{$}}
+    // CHECK-NOT: yield 0
+    // CHECK-NOT: }
+    _read {
+      yield 0
+    }
+
+    // CHECK: @inlinable _modify {
+    // CHECK-NEXT: var x = 0
+    // CHECK-NEXT: yield &x
+    // CHECK-NEXT: }
+    @inlinable _modify {
+      var x = 0
+      yield &x
+    }
+    // CHECK-NEXT: }
+  }
+
+  // CHECK: public var normalReadAndModify: [[INT]] {
+  public var normalReadAndModify: Int {
+    // CHECK-NEXT: _read{{$}}
+    _read { yield 0 }
+    // CHECK-NEXT: _modify{{$}}
+    _modify {
+      var x = 0
+      yield &x
+    }
+    // CHECK-NEXT: }
+  }
+
   // CHECK: @inlinable public func inlinableMethod() {
   // CHECK-NOT: #if NO
   // CHECK-NOT: print("Hello, world!")
