Merge pull request swiftlang#82008 from rjmccall/dont-copy-print-options

Prevent PrintOptions from being implicitly copied

Author: rjmccall

include/swift/AST/ASTPrinter.h

@@ -442,7 +442,7 @@ StringRef getAccessorKindString(AccessorKind value);
 /// may be called multiple times if the declaration uses suppressible
 /// features.
 void printWithCompatibilityFeatureChecks(ASTPrinter &printer,
-                                         PrintOptions &options,
+                                         const PrintOptions &options,
                                          Decl *decl,
                                          llvm::function_ref<void()> printBody);
 

include/swift/AST/GenericSignature.h

@@ -502,8 +502,8 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
                       ArrayRef<GenericTypeParamType *> genericParams,
                       ArrayRef<Requirement> requirements);
 
-  void print(raw_ostream &OS, PrintOptions Options = PrintOptions()) const;
-  void print(ASTPrinter &Printer, PrintOptions Opts = PrintOptions()) const;
+  void print(raw_ostream &OS, const PrintOptions &Options = PrintOptions()) const;
+  void print(ASTPrinter &Printer, const PrintOptions &Opts = PrintOptions()) const;
   SWIFT_DEBUG_DUMP;
   std::string getAsString() const;
 

include/swift/AST/PrintOptions.h

@@ -19,6 +19,7 @@
 #include "swift/Basic/STLExtras.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include <limits.h>
 #include <optional>
 #include <vector>
@@ -72,15 +73,15 @@ class BracketOptions {
                   CloseExtension(CloseExtension),
                   CloseNominal(CloseNominal) {}
 
-  bool shouldOpenExtension(const Decl *D) {
+  bool shouldOpenExtension(const Decl *D) const {
     return D != Target || OpenExtension;
   }
 
-  bool shouldCloseExtension(const Decl *D) {
+  bool shouldCloseExtension(const Decl *D) const {
     return D != Target || CloseExtension;
   }
 
-  bool shouldCloseNominal(const Decl *D) {
+  bool shouldCloseNominal(const Decl *D) const {
     return D != Target || CloseNominal;
   }
 };
@@ -127,7 +128,39 @@ struct ShouldPrintChecker {
 ///
 /// A default-constructed PrintOptions is suitable for printing to users;
 /// there are also factory methods for specific use cases.
+///
+/// The value semantics of PrintOptions are a little messed up. We generally
+/// pass around options by const reference in order to (1) make it
+/// easier to pass in temporaries and (2) discourage direct local mutation
+/// in favor of the OverrideScope system below. However, that override
+/// system assumes that PrintOptions objects are always actually mutable.
 struct PrintOptions {
+
+  /// Explicitly copy these print options. You should generally aim to
+  /// avoid doing this, especially in deeply-embedded code, because
+  /// PrintOptions is a relatively heavyweight type (and is likely to
+  /// only get more heavyweight). Instead, try to use OverrideScope.
+  PrintOptions clone() const { return *this; }
+
+  /// Allow move construction and assignment. We don't expect to
+  /// actually use these much, but there isn't too much harm from
+  /// them.
+  PrintOptions(PrintOptions &&) = default;
+  PrintOptions &operator=(PrintOptions &&) = default;
+
+private:
+  /// Disallow implicit copying, but make it available privately for the
+  /// use of clone().
+  PrintOptions(const PrintOptions &) = default;
+
+  /// Disallow copy assignment completely, which we don't even need
+  /// privately.
+  PrintOptions &operator=(const PrintOptions &) = delete;
+
+public:
+  // defined later in this file
+  class OverrideScope;
+
   /// The indentation width.
   unsigned Indent = 2;
 
@@ -749,6 +782,8 @@ struct PrintOptions {
   void setBaseType(Type T);
 
   void initForSynthesizedExtension(TypeOrExtensionDecl D);
+  void initForSynthesizedExtensionInScope(TypeOrExtensionDecl D,
+                                          OverrideScope &scope) const;
 
   void clearSynthesizedExtension();
 
@@ -823,7 +858,87 @@ struct PrintOptions {
     PO.AlwaysTryPrintParameterLabels = true;
     return PO;
   }
+
+  /// An RAII scope for performing temporary adjustments to a PrintOptions
+  /// object. Even with the abstraction inherent in this design, this can
+  /// be significantly cheaper than copying the options just to modify a few
+  /// fields.
+  ///
+  /// At its core, this is just a stack of arbitrary functions to run
+  /// when the scope is destroyed.
+  class OverrideScope {
+  public:
+    /// The mutable options exposed by the scope. Generally, you should not
+    /// access this directly.
+    PrintOptions &Options;
+
+  private:
+    /// A stack of finalizer functions, each of which generally undoes some
+    /// change that was made to the options.
+    SmallVector<std::function<void(PrintOptions &)>, 4> Finalizers;
+
+  public:
+    OverrideScope(const PrintOptions &options)
+      : Options(const_cast<PrintOptions &>(options)) {}
+
+    // Disallow all copies and moves.
+    OverrideScope(const OverrideScope &scope) = delete;
+    OverrideScope &operator=(const OverrideScope &scope) = delete;
+
+    ~OverrideScope() {
+      // Run the finalizers in the opposite order that they were added.
+      for (auto &finalizer : llvm::reverse(Finalizers)) {
+        finalizer(Options);
+      }
+    }
+
+    template <class Fn>
+    void addFinalizer(Fn &&fn) {
+      Finalizers.emplace_back(std::move(fn));
+    }
+
+    void addExcludedAttr(AnyAttrKind kind) {
+      Options.ExcludeAttrList.push_back(kind);
+      addFinalizer([](PrintOptions &options) {
+        options.ExcludeAttrList.pop_back();
+      });
+    }
+  };
 };
-}
+
+/// Override a print option within an OverrideScope. Does a check to see if
+/// the new value is the same as the old before actually doing anything, so
+/// it only works if the type provides ==.
+///
+/// Signature is:
+///   void (OverrideScope &scope, <FIELD NAME>, T &&newValue)
+#define OVERRIDE_PRINT_OPTION(SCOPE, FIELD_NAME, VALUE)                     \
+  do {                                                                      \
+    auto _newValue = (VALUE);                                               \
+    if ((SCOPE).Options.FIELD_NAME != _newValue) {                          \
+      auto finalizer =                                                      \
+        [_oldValue=(SCOPE).Options.FIELD_NAME](PrintOptions &opts) {        \
+          opts.FIELD_NAME = _oldValue;                                      \
+        };                                                                  \
+      (SCOPE).Options.FIELD_NAME = std::move(_newValue);                    \
+      (SCOPE).addFinalizer(std::move(finalizer));                           \
+    }                                                                       \
+  } while(0)
+
+/// Override a print option within an OverrideScope. Works for any type.
+///
+/// Signature is:
+///   void (OverrideScope &scope, <FIELD NAME>, T &&newValue)
+#define OVERRIDE_PRINT_OPTION_UNCONDITIONAL(SCOPE, FIELD_NAME, VALUE)       \
+  do {                                                                      \
+    auto finalizer =                                                        \
+      [_oldValue=(SCOPE).Options.FIELD_NAME](PrintOptions &opts) {          \
+        opts.FIELD_NAME = _oldValue;                                        \
+      };                                                                    \
+    (SCOPE).Options.FIELD_NAME = (VALUE);                                   \
+    (SCOPE).addFinalizer(std::move(finalizer));                             \
+  } while(0)
+
+} // end namespace swift
 
 #endif // LLVM_SWIFT_AST_PRINTOPTIONS_H

include/swift/Sema/ConstraintGraph.h

@@ -216,7 +216,7 @@ class ConstraintGraphNode {
 
   /// Print this graph node.
   void print(llvm::raw_ostream &out, unsigned indent,
-             PrintOptions PO = PrintOptions()) const;
+             const PrintOptions &PO = PrintOptions()) const;
 
   SWIFT_DEBUG_DUMP;
 

include/swift/Sema/IDETypeChecking.h

@@ -197,7 +197,7 @@ namespace swift {
     Implementation &Impl;
   public:
     SynthesizedExtensionAnalyzer(NominalTypeDecl *Target,
-                                 PrintOptions Options,
+                                 PrintOptions &&Options,
                                  bool IncludeUnconditional = true);
     ~SynthesizedExtensionAnalyzer();
 

lib/AST/ASTDumper.cpp

@@ -1720,7 +1720,8 @@ namespace {
     /// Prints a type as a field. If writing a parsable output format, the
     /// `PrintOptions` are ignored and the type is written as a USR; otherwise,
     /// the type is stringified using the `PrintOptions`.
-    void printTypeField(Type Ty, Label label, PrintOptions opts = PrintOptions(),
+    void printTypeField(Type Ty, Label label,
+                        const PrintOptions &opts = PrintOptions(),
                         TerminalColor Color = TypeColor) {
       if (Writer.isParsable()) {
         printField(typeUSR(Ty), label, Color);