Merge pull request #29984 from dan-zheng/TF-1168

Author: swift-ci

lib/Parse/ParseDecl.cpp

@@ -1176,8 +1176,16 @@ ParserResult<DerivativeAttr> Parser::parseDerivativeAttribute(SourceLoc atLoc,
   SmallVector<ParsedAutoDiffParameter, 8> parameters;
 
   // Parse trailing comma, if it exists, and check for errors.
-  auto consumeIfTrailingComma = [&]() -> bool {
-    if (!consumeIf(tok::comma)) return false;
+  auto consumeIfTrailingComma = [&](bool requireComma = false) -> bool {
+    if (!consumeIf(tok::comma)) {
+      // If comma is required but does not exist and ')' has not been reached,
+      // diagnose missing comma.
+      if (requireComma && !Tok.is(tok::r_paren)) {
+        diagnose(getEndOfPreviousLoc(), diag::expected_separator, ",");
+        return true;
+      }
+      return false;
+    }
     // Diagnose trailing comma before ')'.
     if (Tok.is(tok::r_paren)) {
       diagnose(Tok, diag::unexpected_separator, ",");
@@ -1211,7 +1219,7 @@ ParserResult<DerivativeAttr> Parser::parseDerivativeAttribute(SourceLoc atLoc,
               baseType, original))
         return makeParserError();
     }
-    if (consumeIfTrailingComma())
+    if (consumeIfTrailingComma(/*requireComma*/ true))
       return makeParserError();
     // Parse the optional 'wrt' differentiability parameters clause.
     if (isIdentifier(Tok, "wrt") &&
@@ -1250,8 +1258,16 @@ ParserResult<TransposeAttr> Parser::parseTransposeAttribute(SourceLoc atLoc,
   SmallVector<ParsedAutoDiffParameter, 8> parameters;
 
   // Parse trailing comma, if it exists, and check for errors.
-  auto consumeIfTrailingComma = [&]() -> bool {
-    if (!consumeIf(tok::comma)) return false;
+  auto consumeIfTrailingComma = [&](bool requireComma = false) -> bool {
+    if (!consumeIf(tok::comma)) {
+      // If comma is required but does not exist and ')' has not been reached,
+      // diagnose missing comma.
+      if (requireComma && !Tok.is(tok::r_paren)) {
+        diagnose(Tok, diag::expected_separator, ",");
+        return true;
+      }
+      return false;
+    }
     // Diagnose trailing comma before ')'.
     if (Tok.is(tok::r_paren)) {
       diagnose(Tok, diag::unexpected_separator, ",");
@@ -1286,7 +1302,7 @@ ParserResult<TransposeAttr> Parser::parseTransposeAttribute(SourceLoc atLoc,
               baseType, original))
         return makeParserError();
     }
-    if (consumeIfTrailingComma())
+    if (consumeIfTrailingComma(/*requireComma*/ true))
       return makeParserError();
     // Parse the optional 'wrt' linearity parameters clause.
     if (Tok.is(tok::identifier) && Tok.getText() == "wrt" &&

test/AutoDiff/Parse/derivative_attr_parse.swift

@@ -85,7 +85,15 @@ func dfoo(x: Float) -> (value: Float, differential: (Float) -> (Float)) {
   return (x, { $0 })
 }
 
-func localDerivativeRegistration() {
+// TF-1168: missing comma before `wrt:`.
+// expected-error @+2 {{expected ',' separator}}
+// expected-error @+1 {{expected declaration}}
+@derivative(of: foo wrt: x)
+func dfoo(x: Float) -> (value: Float, differential: (Float) -> (Float)) {
+  return (x, { $0 })
+}
+
+func testLocalDerivativeRegistration() {
   // expected-error @+1 {{attribute '@derivative' can only be used in a non-local scope}}
   @derivative(of: sin)
   func dsin()

test/AutoDiff/Parse/transpose_attr_parse.swift

@@ -78,16 +78,28 @@ func transpose(v: Float) -> Float
 @transpose(of: foo, wrt: (0, v))
 func transpose(v: Float) -> Float
 
-// expected-error @+2 {{expected ')' in 'transpose' attribute}}
+// NOTE: The "expected ',' separator" diagnostic is not ideal.
+// Ideally, the diagnostic should point out that that `Swift.Float.+(_:_)` is
+// not a valid declaration name (missing colon after second argument label).
+// expected-error @+2 {{expected ',' separator}}
 // expected-error @+1 {{expected declaration}}
 @transpose(of: Swift.Float.+(_:_))
 func transpose(v: Float) -> Float
 
-// expected-error @+2 {{expected ')' in 'transpose' attribute}}
+// NOTE: The "expected ',' separator" diagnostic is not ideal.
+// Ideally, the diagnostic should point out that that `Swift.Float.+.a` is
+// not a valid declaration name.
+// expected-error @+2 {{expected ',' separator}}
 // expected-error @+1 {{expected declaration}}
 @transpose(of: Swift.Float.+.a)
 func transpose(v: Float) -> Float
 
+// TF-1168: missing comma before `wrt:`.
+// expected-error @+2 {{expected ',' separator}}
+// expected-error @+1 {{expected declaration}}
+@transpose(of: foo wrt: x)
+func transpose(v: Float) -> Float
+
 func testLocalTransposeRegistration() {
   // Transpose registration can only be non-local.
   // expected-error @+1 {{attribute '@transpose' can only be used in a non-local scope}}