Remove @transposing attribute.

dan-zheng · dan-zheng · commit ef832ceaeeb6 · 2019-11-26T14:44:26.000-08:00
`@transposing` attribute has no known users, so deleting it outright makes
maintenance easier.

Other assorted gardening included.
diff --git a/include/swift/AST/Attr.def b/include/swift/AST/Attr.def
@@ -545,11 +545,6 @@ DECL_ATTR(differentiating, Differentiating,
   OnFunc | LongAttribute | AllowMultipleAttributes |
   ABIStableToAdd | ABIBreakingToRemove | APIStableToAdd | APIBreakingToRemove |
   NotSerialized, 98)
-// TODO(TF-999): Remove deprecated `@transposing` attribute.
-DECL_ATTR(transposing, Transposing,
-  OnFunc | LongAttribute | AllowMultipleAttributes |
-  ABIStableToAdd | ABIBreakingToRemove | APIStableToAdd | APIBreakingToRemove |
-  NotSerialized, 99)
 // SWIFT_ENABLE_TENSORFLOW END
 
 #undef TYPE_ATTR
diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h
@@ -1760,7 +1760,7 @@ class TransposeAttr final
   AbstractFunctionDecl *OriginalFunction = nullptr;
   /// The number of parsed parameters specified in 'wrt:'.
   unsigned NumParsedParameters = 0;
-  /// The differentiation parameters' indices, resolved by the type checker.
+  /// The transposed parameters' indices, resolved by the type checker.
   IndexSubset *ParameterIndices = nullptr;
 
   explicit TransposeAttr(bool implicit, SourceLoc atLoc, SourceRange baseRange,
@@ -1793,8 +1793,8 @@ class TransposeAttr final
     OriginalFunction = decl;
   }
 
-  /// The parsed transposing parameters, i.e. the list of parameters
-  /// specified in 'wrt:'.
+  /// The parsed transposed parameters, i.e. the list of parameters specified in
+  /// 'wrt:'.
   ArrayRef<ParsedAutoDiffParameter> getParsedParameters() const {
     return {getTrailingObjects<ParsedAutoDiffParameter>(), NumParsedParameters};
   }
@@ -1817,9 +1817,6 @@ class TransposeAttr final
   }
 };
 
-// TODO(TF-999): Remove deprecated `@transposing` attribute.
-using TransposingAttr = TransposeAttr;
-
 /// Relates a property to its projection value property, as described by a property wrapper. For
 /// example, given
 /// \code
diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def
@@ -1577,12 +1577,9 @@ ERROR(attr_transpose_expected_original_name,PointsToFirstBadToken,
       "expected an original function name", ())
 ERROR(attr_transpose_expected_label_linear_or_wrt,none,
       "expected 'wrt:'", ())
-WARNING(attr_transposing_deprecated,PointsToFirstBadToken,
-        "'@transposing' attribute is deprecated; use '@transpose(of:)' instead",
-        ())
 
-// transposing `wrt` parameters clause
-ERROR(transposing_params_clause_expected_parameter,PointsToFirstBadToken,
+// transpose `wrt` parameters clause
+ERROR(transpose_params_clause_expected_parameter,PointsToFirstBadToken,
       "expected a parameter, which can be a 'unsigned int' parameter number "
       "or 'self'", ())
 
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -996,13 +996,10 @@ class Parser {
       Optional<DeclNameWithLoc> &jvpSpec, Optional<DeclNameWithLoc> &vjpSpec,
       TrailingWhereClause *&whereClause);
 
-  /// Parse a differentiation parameters clause.
+  /// Parse a differentiation parameters clause, i.e. the "wrt:" clause in
+  /// @differentiable and @derivative attributes.
   bool parseDifferentiationParametersClause(
       SmallVectorImpl<ParsedAutoDiffParameter> &params, StringRef attrName);
-  
-  /// Parse a transposing parameters clause.
-  bool parseTransposingParametersClause(
-      SmallVectorImpl<ParsedAutoDiffParameter> &params, StringRef attrName);
 
   /// Parse the @derivative attribute.
   ParserResult<DerivativeAttr> parseDerivativeAttribute(SourceLoc AtLoc,
@@ -1013,15 +1010,15 @@ class Parser {
   ParserResult<DerivativeAttr> parseDifferentiatingAttribute(SourceLoc AtLoc,
                                                              SourceLoc Loc);
 
+  /// Parse a transposed parameters clause, i.e. the "wrt:" clause in @transpose
+  /// attributes.
+  bool parseTransposedParametersClause(
+      SmallVectorImpl<ParsedAutoDiffParameter> &params, StringRef attrName);
+
   /// Parse the @transpose attribute.
   ParserResult<TransposeAttr> parseTransposeAttribute(SourceLoc AtLoc,
                                                       SourceLoc Loc);
 
-  /// Parse the deprecated @transposing attribute.
-  // TODO(TF-999): Remove the deprecated `@transposing` attribute.
-  ParserResult<TransposeAttr> parseTransposingAttribute(SourceLoc AtLoc,
-                                                        SourceLoc Loc);
-
   /// Parse the @quoted attribute.
   ParserResult<QuotedAttr> parseQuotedAttribute(SourceLoc AtLoc, SourceLoc Loc);
 
diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp
@@ -936,10 +936,10 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
   }
 
   // SWIFT_ENABLE_TENSORFLOW
-  case DAK_Transposing: {
-    Printer.printAttrName("@transposing");
+  case DAK_Transpose: {
+    Printer.printAttrName("@transpose");
     Printer << '(';
-    auto *attr = cast<TransposingAttr>(this);
+    auto *attr = cast<TransposeAttr>(this);
     auto *transpose = cast<AbstractFunctionDecl>(D);
     Printer << attr->getOriginalFunctionName().Name;
     auto transParamsString = getTransposedParametersClauseString(
@@ -1114,10 +1114,9 @@ StringRef DeclAttribute::getAttrName() const {
     return "transpose";
   case DAK_Differentiating:
     return "differentiating";
-  case DAK_Transposing:
-    return "transposing";
   case DAK_Quoted:
     return "quoted";
+  // SWIFT_ENABLE_TENSORFLOW END
   }
   llvm_unreachable("bad DeclAttrKind");
 }
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -929,7 +929,7 @@ bool Parser::parseDifferentiationParametersClause(
   return false;
 }
 
-bool Parser::parseTransposingParametersClause(
+bool Parser::parseTransposedParametersClause(
     SmallVectorImpl<ParsedAutoDiffParameter> &params, StringRef attrName) {
   SyntaxParsingContext TransposeParamsClauseContext(
       SyntaxContext, SyntaxKind::DifferentiationParamsClause);
@@ -950,7 +950,7 @@ bool Parser::parseTransposingParametersClause(
         unsigned int paramNum;
         if (parseUnsignedInteger(
                 paramNum, paramLoc,
-                diag::transposing_params_clause_expected_parameter))
+                diag::transpose_params_clause_expected_parameter))
           return true;
 
         params.push_back(
@@ -963,7 +963,7 @@ bool Parser::parseTransposingParametersClause(
         break;
       }
       default:
-        diagnose(Tok, diag::transposing_params_clause_expected_parameter);
+        diagnose(Tok, diag::transpose_params_clause_expected_parameter);
         return true;
     }
     if (parseTrailingComma && Tok.isNot(tok::r_paren))
@@ -1362,73 +1362,9 @@ ParserResult<TransposeAttr> Parser::parseTransposeAttribute(SourceLoc atLoc,
       if (consumeIfTrailingComma())
         return makeParserError();
     }
-    // Parse the optional 'wrt' differentiation parameters clause.
-    if (Tok.is(tok::identifier) && Tok.getText() == "wrt" &&
-        parseTransposingParametersClause(params, AttrName))
-      return makeParserError();
-  }
-  // Parse ')'.
-  if (!consumeIf(tok::r_paren, rParenLoc)) {
-    diagnose(getEndOfPreviousLoc(), diag::attr_expected_rparen, AttrName,
-             /*DeclModifier*/ false);
-    return makeParserError();
-  }
-  return ParserResult<TransposeAttr>(TransposeAttr::create(
-      Context, /*implicit*/ false, atLoc, SourceRange(loc, rParenLoc), baseType,
-      original, params));
-}
-
-ParserResult<TransposeAttr> Parser::parseTransposingAttribute(SourceLoc atLoc,
-                                                              SourceLoc loc) {
-  StringRef AttrName = "transposing";
-  SourceLoc lParenLoc = loc, rParenLoc = loc;
-  TypeRepr *baseType;
-  DeclNameWithLoc original;
-  SmallVector<ParsedAutoDiffParameter, 8> params;
-
-  // Parse trailing comma, if it exists, and check for errors.
-  auto consumeIfTrailingComma = [&]() -> bool {
-    if (!consumeIf(tok::comma))
-      return false;
-    // Diagnose trailing comma before ')'.
-    if (Tok.is(tok::r_paren)) {
-      diagnose(Tok, diag::unexpected_separator, ",");
-      return errorAndSkipUntilConsumeRightParen(*this, AttrName);
-    }
-    // Check that token after comma is 'wrt:'.
-    if (isIdentifier(Tok, "wrt"))
-      return false;
-    diagnose(Tok, diag::attr_expected_label, "wrt", AttrName);
-    return errorAndSkipUntilConsumeRightParen(*this, AttrName);
-  };
-
-  // Parse '('.
-  if (!consumeIf(tok::l_paren, lParenLoc)) {
-    diagnose(getEndOfPreviousLoc(), diag::attr_expected_lparen, AttrName,
-             /*DeclModifier*/ false);
-    return makeParserError();
-  }
-  {
-    SyntaxParsingContext ContentContext(
-        SyntaxContext,
-        SyntaxKind::DeprecatedDerivativeRegistrationAttributeArguments);
-    {
-      // Parse the optionally qualified function name.
-      // TODO(TF-1009): Fix syntax support for dot-separated qualified names.
-      // Currently, `SyntaxKind::FunctionDeclName` only supports unqualified
-      // names.
-      SyntaxParsingContext FuncDeclNameContext(SyntaxContext,
-                                               SyntaxKind::FunctionDeclName);
-      if (parseQualifiedDeclName(*this,
-                                 diag::attr_transpose_expected_original_name,
-                                 baseType, original))
-        return makeParserError();
-      if (consumeIfTrailingComma())
-        return makeParserError();
-    }
-    // Parse the optional 'wrt' differentiation parameters clause.
+    // Parse the optional 'wrt' transposed parameters clause.
     if (Tok.is(tok::identifier) && Tok.getText() == "wrt" &&
-        parseTransposingParametersClause(params, AttrName))
+        parseTransposedParametersClause(params, AttrName))
       return makeParserError();
   }
   // Parse ')'.
@@ -2308,21 +2244,6 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
     break;
   }
 
-  // SWIFT_ENABLE_TENSORFLOW
-  case DAK_Transposing: {
-    // Diagnose deprecated `@transposing` attribute.
-    diagnose(Loc, diag::attr_transposing_deprecated);
-
-    // `@transposing` in a local scope is not allowed.
-    if (CurDeclContext->isLocalContext())
-      diagnose(Loc, diag::attr_only_at_non_local_scope, '@' + AttrName.str());
-
-    auto Attr = parseTransposingAttribute(AtLoc, Loc);
-    if (Attr.isNonNull())
-      Attributes.add(Attr.get());
-    break;
-  }
-
   case DAK_ProjectedValueProperty: {
     if (!consumeIf(tok::l_paren)) {
       diagnose(Loc, diag::attr_expected_lparen, AttrName,
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -252,8 +252,6 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   void visitTransposeAttr(TransposeAttr *attr);
   // TODO(TF-999): Remove deprecated `@differentiating` attribute.
   void visitDifferentiatingAttr(DerivativeAttr *attr);
-  // TODO(TF-999): Remove deprecated `@transposing` attribute.
-  void visitTransposingAttr(TransposeAttr *attr);
   void visitCompilerEvaluableAttr(CompilerEvaluableAttr *attr);
   void visitNoDerivativeAttr(NoDerivativeAttr *attr);
   // SWIFT_ENABLE_TENSORFLOW END
@@ -3094,15 +3092,13 @@ static IndexSubset *computeDifferentiationParameters(
 }
 
 // SWIFT_ENABLE_TENSORFLOW
-// Computes `IndexSubset` from the given parsed transposing parameters
-// (possibly empty) for the given function, then verifies that the parameter
-// indices are valid.
-// The attribute name/location are used in diagnostics.
-static IndexSubset *computeTransposingParameters(
+// Computes `IndexSubset` from the given parsed transposed parameters (possibly
+// empty) for the given function, then verifies that the parameter indices are
+// valid. The attribute name/location are used in diagnostics.
+static IndexSubset *computeTransposedParameters(
     ArrayRef<ParsedAutoDiffParameter> parsedWrtParams,
     AbstractFunctionDecl *transposeFunction, bool isCurried,
-    GenericEnvironment *derivativeGenEnv, SourceLoc attrLoc
-) {
+    GenericEnvironment *derivativeGenEnv, SourceLoc attrLoc) {
   auto &ctx = transposeFunction->getASTContext();
   auto &diags = ctx.Diags;
 
@@ -3248,11 +3244,10 @@ static bool checkDifferentiationParameters(
 // context. Returns true on error.
 // The parsed differentiation parameters and attribute location are used in
 // diagnostics.
-static bool checkTransposingParameters(
-    AbstractFunctionDecl *AFD,
-    SmallVector<Type, 4> wrtParamTypes, GenericEnvironment *derivativeGenEnv,
-    ModuleDecl *module, ArrayRef<ParsedAutoDiffParameter> parsedWrtParams,
-    SourceLoc attrLoc) {
+static bool checkTransposedParameters(
+    AbstractFunctionDecl *AFD, SmallVector<Type, 4> wrtParamTypes,
+    GenericEnvironment *derivativeGenEnv, ModuleDecl *module,
+    ArrayRef<ParsedAutoDiffParameter> parsedWrtParams, SourceLoc attrLoc) {
   auto &ctx = AFD->getASTContext();
   auto &diags = ctx.Diags;
 
@@ -4001,17 +3996,17 @@ void AttributeChecker::visitTransposeAttr(TransposeAttr *attr) {
   // If checked wrt param indices are not specified, compute them.
   bool isCurried = transposeInterfaceType->getResult()->is<AnyFunctionType>();
   if (!wrtParamIndices)
-    wrtParamIndices = computeTransposingParameters(
+    wrtParamIndices = computeTransposedParameters(
         parsedWrtParams, transpose, isCurried,
         transpose->getGenericEnvironment(), attr->getLocation());
   if (!wrtParamIndices) {
     D->getAttrs().removeAttribute(attr);
     attr->setInvalid();
     return;
   }
-  
+
   // Diagnose empty parameter indices. This occurs when no `wrt` clause is
-  // declared and no differentiation parameters can be inferred.
+  // declared and no transposed parameters can be inferred.
   if (wrtParamIndices->isEmpty()) {
     diagnose(attr->getLocation(),
              diag::diff_params_clause_no_inferred_parameters);
@@ -4104,8 +4099,8 @@ void AttributeChecker::visitTransposeAttr(TransposeAttr *attr) {
              originalName.Name);
   };
 
-  // Returns true if the derivative function and original function candidate are
-  // defined in compatible type contexts. If the derivative function and the
+  // Returns true if the transpose function and original function candidate are
+  // defined in compatible type contexts. If the transpose function and the
   // original function candidate have different parents, return false.
   std::function<bool(AbstractFunctionDecl *)> hasValidTypeContext =
       [&](AbstractFunctionDecl *decl) { return true; };
@@ -4138,34 +4133,29 @@ void AttributeChecker::visitTransposeAttr(TransposeAttr *attr) {
 
   attr->setOriginalFunction(originalAFD);
 
-  // Gather differentiation parameters.
-  // Differentiation parameters are with respect to the original function.
+  // Get the transposed parameter types.
   SmallVector<Type, 4> wrtParamTypes;
   autodiff::getSubsetParameterTypes(wrtParamIndices, expectedOriginalFnType,
                                     wrtParamTypes);
 
-  // Check if differentiation parameter indices are valid.
-  if (checkTransposingParameters(originalAFD, wrtParamTypes,
-                                 transpose->getGenericEnvironment(),
-                                 transpose->getModuleContext(), parsedWrtParams,
-                                 attr->getLocation())) {
+  // Check if transposed parameter indices are valid.
+  if (checkTransposedParameters(originalAFD, wrtParamTypes,
+                                transpose->getGenericEnvironment(),
+                                transpose->getModuleContext(), parsedWrtParams,
+                                attr->getLocation())) {
     D->getAttrs().removeAttribute(attr);
     attr->setInvalid();
     return;
   }
 
-  // Set the checked differentiation parameter indices in the attribute.
+  // Set the checked transposed parameter indices in the attribute.
   attr->setParameterIndices(wrtParamIndices);
 }
 
 void AttributeChecker::visitDifferentiatingAttr(DerivativeAttr *attr) {
   visitDerivativeAttr(attr);
 }
 
-void AttributeChecker::visitTransposingAttr(TransposeAttr *attr) {
-  visitTransposeAttr(attr);
-}
-
 static bool
 compilerEvaluableAllowedInExtensionDecl(ExtensionDecl *extensionDecl) {
   auto extendedTypeKind = extensionDecl->getExtendedType()->getKind();
diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp
@@ -1429,7 +1429,6 @@ namespace  {
     UNINTERESTING_ATTR(Derivative)
     UNINTERESTING_ATTR(Transpose)
     UNINTERESTING_ATTR(Differentiating)
-    UNINTERESTING_ATTR(Transposing)
     UNINTERESTING_ATTR(NoDerivative)
     UNINTERESTING_ATTR(CompilerEvaluable)
     // SWIFT_ENABLE_TENSORFLOW END
diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h
@@ -1792,9 +1792,6 @@ namespace decls_block {
     BCArray<BCFixed<1>> // Differentiation parameter indices' bitvector.
   >;
 
-  // TODO(TF-999): Remove deprecated `@transposing` attribute.
-  using TransposingDeclAttrLayout = TransposeDeclAttrLayout;
-
 #define SIMPLE_DECL_ATTR(X, CLASS, ...) \
   using CLASS##DeclAttrLayout = BCRecordLayout< \
     CLASS##_DECL_ATTR, \
diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp
@@ -2127,7 +2127,6 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     case DAK_Derivative:
     case DAK_Transpose:
     case DAK_Differentiating:
-    case DAK_Transposing:
       llvm_unreachable("cannot serialize attribute");
     // SWIFT_ENABLE_TENSORFLOW END
 
diff --git a/test/AutoDiff/transpose_attr_parse.swift b/test/AutoDiff/transpose_attr_parse.swift
diff --git a/test/IDE/complete_decl_attribute.swift b/test/IDE/complete_decl_attribute.swift
diff --git a/test/Syntax/Outputs/round_trip_parse_gen.swift.withkinds b/test/Syntax/Outputs/round_trip_parse_gen.swift.withkinds
diff --git a/test/Syntax/round_trip_parse_gen.swift b/test/Syntax/round_trip_parse_gen.swift
