Remove `requiresLeadingNewline` from basic format #1690

kimdv · 2023-05-22T19:45:42Z

As part of this comment I started by removing the first part

#1623 (comment)

kimdv · 2023-05-22T19:46:30Z

@swift-ci please test

ahoppen

Whooo 🎉

ahoppen · 2023-05-22T20:02:09Z

@swift-ci Please test

kimdv · 2023-05-23T04:08:50Z

@swift-ci please test Windows

kimdv · 2023-05-23T04:10:15Z

@swift-ci please test macOS

kimdv · 2023-05-23T06:22:15Z

@swift-ci please test macOS

kimdv · 2023-05-23T13:56:31Z

I see that those two test fail

  Swift(macosx-x86_64) :: Macros/macro_expand.swift
  Swift(macosx-x86_64) :: SourceKit/Macros/diags.swift

I got one of the errors and it looks like we don't get the expected output.
From what I can see we now get precedencegroup MyPrecedence {} and before precedencegroup MyPrecedence {\n}.

Is this okay? If yes, how can I update the failing test cases.

SourceKit/Macros/diags.swift

******************** TEST 'Swift(macosx-x86_64) :: SourceKit/Macros/diags.swift' FAILED ********************
Script:
--
: 'RUN: at line 21';   rm -rf "/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp" && mkdir -p "/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp"
: 'RUN: at line 24';   env SDKROOT=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk '/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/bin/swiftc' -toolchain-stdlib-rpath -Xlinker -rpath -Xlinker /usr/lib/swift -module-cache-path /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache -swift-version 4  -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 9999:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.0:macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.1:macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.2:macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.3:macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.4:macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.5:macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.6:macOS 12.3, iOS 15.4, watchOS 8.5, tvOS 15.4' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.7:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.8:macOS 13.3, iOS 16.4, watchOS 9.4, tvOS 16.4' -Xfrontend -define-availability -Xfrontend 'SwiftStdlib 5.9:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999' -sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk -target x86_64-apple-macosx10.13 -I /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/lib/swift/host -L /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/lib/swift/host -swift-version 5 -emit-library -o /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp/libMacroDefinition.dylib -module-name=MacroDefinition /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/../../Macros/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath
: 'RUN: at line 26';   /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/bin/sourcekitd-test -module-cache-path '/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache' -req=diags /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/diags.swift -- -swift-version 5 -load-plugin-library /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp/libMacroDefinition.dylib -module-name MacroUser /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/diags.swift | /Applications/Xcode.app/Contents/Developer/usr/bin/python3 /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Inputs/sourcekitd_path_sanitize.py > /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp.response
: 'RUN: at line 27';   diff -u /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/diags.swift.response /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp.response
--
Exit Code: 1

Command Output (stdout):
--
--- /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/diags.swift.response	2023-05-23 06:24:27.000000000 +0000
+++ /Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp.response	2023-05-23 07:12:58.000000000 +0000
@@ -37,7 +37,7 @@
       key.buffer_name: "@__swiftmacro_9MacroUser3fooyyF11coerceToIntfMf_.swift"
     },
     {
-      key.buffer_text: "import Swift\n\nprecedencegroup MyPrecedence {}\n\n@attached(member) macro myMacro()\n\nextension Int {\n}\n\n@main\nstruct MyMain {\n  static func main() {\n  }\n}\n\ntypealias Array = Void\n\ntypealias Dictionary = Void\n\ntypealias BooleanLiteralType = Void\n\ntypealias ExtendedGraphemeClusterType = Void\n\ntypealias FloatLiteralType = Void\n\ntypealias IntegerLiteralType = Void\n\ntypealias StringLiteralType = Void\n\ntypealias UnicodeScalarType = Void\n\ntypealias _ColorLiteralType = Void\n\ntypealias _ImageLiteralType = Void\n\ntypealias _FileReferenceLiteralType = Void",
+      key.buffer_text: "import Swift\n\nprecedencegroup MyPrecedence {\n}\n\n@attached(member) macro myMacro()\n\nextension Int {\n}\n\n@main\nstruct MyMain {\n  static func main() {\n  }\n}\n\ntypealias Array = Void\n\ntypealias Dictionary = Void\n\ntypealias BooleanLiteralType = Void\n\ntypealias ExtendedGraphemeClusterType = Void\n\ntypealias FloatLiteralType = Void\n\ntypealias IntegerLiteralType = Void\n\ntypealias StringLiteralType = Void\n\ntypealias UnicodeScalarType = Void\n\ntypealias _ColorLiteralType = Void\n\ntypealias _ImageLiteralType = Void\n\ntypealias _FileReferenceLiteralType = Void",
       key.original_location: {
         key.offset: 499,
         key.length: 0,
@@ -200,7 +200,7 @@
       ]
     },
     {
-      key.line: 5,
+      key.line: 6,
       key.column: 25,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -224,7 +224,7 @@
       ]
     },
     {
-      key.line: 5,
+      key.line: 6,
       key.column: 25,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -248,7 +248,7 @@
       ]
     },
     {
-      key.line: 7,
+      key.line: 8,
       key.column: 1,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -272,7 +272,7 @@
       ]
     },
     {
-      key.line: 10,
+      key.line: 11,
       key.column: 1,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -296,7 +296,7 @@
       ]
     },
     {
-      key.line: 11,
+      key.line: 12,
       key.column: 8,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -320,7 +320,7 @@
       ]
     },
     {
-      key.line: 16,
+      key.line: 17,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -344,7 +344,7 @@
       ]
     },
     {
-      key.line: 18,
+      key.line: 19,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -368,7 +368,7 @@
       ]
     },
     {
-      key.line: 20,
+      key.line: 21,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -392,7 +392,7 @@
       ]
     },
     {
-      key.line: 22,
+      key.line: 23,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -416,7 +416,7 @@
       ]
     },
     {
-      key.line: 24,
+      key.line: 25,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -440,7 +440,7 @@
       ]
     },
     {
-      key.line: 26,
+      key.line: 27,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -464,7 +464,7 @@
       ]
     },
     {
-      key.line: 28,
+      key.line: 29,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -488,7 +488,7 @@
       ]
     },
     {
-      key.line: 30,
+      key.line: 31,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -512,7 +512,7 @@
       ]
     },
     {
-      key.line: 32,
+      key.line: 33,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -536,7 +536,7 @@
       ]
     },
     {
-      key.line: 34,
+      key.line: 35,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,
@@ -560,7 +560,7 @@
       ]
     },
     {
-      key.line: 36,
+      key.line: 37,
       key.column: 11,
       key.filepath: "@__swiftmacro_9MacroUser3Bad7InvalidfMp_.swift",
       key.severity: source.diagnostic.severity.error,

--
Command Output (stderr):
--
{
  key.request: source.request.diagnostics,
  key.compilerargs: [
    "-module-cache-path",
    "/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.13/clang-module-cache",
    "-swift-version",
    "5",
    "-load-plugin-library",
    "/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/SourceKit/Macros/Output/diags.swift.tmp/libMacroDefinition.dylib",
    "-module-name",
    "MacroUser",
    "/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/diags.swift"
  ],
  key.sourcefile: "/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/diags.swift",
  key.primary_file: "/Users/ec2-user/jenkins/workspace/swift-syntax-PR-macOS/branch-main/swift/test/SourceKit/Macros/diags.swift"
}

--

********************

ahoppen · 2023-05-23T18:34:13Z

I prefer precedencegroup MyPrecedence {} over precedencegroup MyPrecedence {\n}, so updating the tests seems good to me.

To fix the test failures for SourceKit/Macros/diags.swift, you need to modify the following file https://github.com/apple/swift/blob/main/test/SourceKit/Macros/diags.swift.response. The test that is being run is in the compiler repo as well: https://github.com/apple/swift/blob/main/test/SourceKit/Macros/diags.swift

For Macros/macro_expand.swift, I’m not sure because I don’t see the failure anymore but you most likely need to modify some of the CHECK lines in https://github.com/apple/swift/blob/main/test/Macros/macro_expand.swift

kimdv · 2023-05-23T19:10:13Z

@swift-ci please test

kimdv · 2023-05-23T19:10:47Z

@ahoppen it looks like if we want {} there was more changes.

kimdv · 2023-05-23T20:05:09Z

@swift-ci please test

Sources/SwiftBasicFormat/BasicFormat.swift

kimdv · 2023-05-24T11:28:42Z

@swift-ci please test

kimdv · 2023-05-24T16:54:59Z

@swift-ci please test

ahoppen

Nice to see yet another use of CodeGeneration gone

kimdv · 2023-05-24T19:33:27Z

swiftlang/swift#66106

@swift-ci please test

kimdv · 2023-05-25T05:54:17Z

swiftlang/swift#66106

@swift-ci please test

kimdv · 2023-05-25T06:38:34Z

@swift-ci please test windows

kimdv · 2023-05-29T19:10:26Z

@swift-ci please test

kimdv · 2023-05-30T05:21:25Z

swiftlang/swift#66106

@swift-ci please test

kimdv · 2023-05-30T07:54:10Z

swiftlang/swift#66106

@swift-ci please test

kimdv · 2023-05-30T10:33:23Z

@ahoppen after some tried I got it working.

Will you also review: swiftlang/swift#66106

kimdv requested a review from ahoppen as a code owner May 22, 2023 19:45

ahoppen approved these changes May 22, 2023

View reviewed changes

kimdv force-pushed the kimdv/remove-newline-from-code-gen branch from e5d90e4 to db29a03 Compare May 23, 2023 13:53

kimdv force-pushed the kimdv/remove-newline-from-code-gen branch from db29a03 to 1443953 Compare May 23, 2023 19:05

ahoppen reviewed May 23, 2023

View reviewed changes

Sources/SwiftBasicFormat/BasicFormat.swift Outdated Show resolved Hide resolved

Sources/SwiftBasicFormat/BasicFormat.swift Outdated Show resolved Hide resolved

kimdv force-pushed the kimdv/remove-newline-from-code-gen branch 2 times, most recently from aa2aa98 to a15bbc5 Compare May 24, 2023 11:28

kimdv force-pushed the kimdv/remove-newline-from-code-gen branch from a15bbc5 to aa09b13 Compare May 24, 2023 16:21

ahoppen approved these changes May 24, 2023

View reviewed changes

kimdv mentioned this pull request May 24, 2023

[SwiftSyntax] Remove requiresLeadingNewline from basic format swiftlang/swift#66106

Merged

kimdv force-pushed the kimdv/remove-newline-from-code-gen branch 2 times, most recently from 607b94f to 585a13f Compare May 25, 2023 05:47

Remove requiresLeadingNewline from basic format

b95d65f

kimdv force-pushed the kimdv/remove-newline-from-code-gen branch from 585a13f to b95d65f Compare May 29, 2023 19:01

kimdv merged commit 707c617 into swiftlang:main May 30, 2023

kimdv deleted the kimdv/remove-newline-from-code-gen branch May 30, 2023 19:33

Remove requiresLeadingNewline from basic format #1690

Remove requiresLeadingNewline from basic format #1690

Uh oh!

Conversation

kimdv commented May 22, 2023

Uh oh!

kimdv commented May 22, 2023

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

ahoppen commented May 22, 2023

Uh oh!

kimdv commented May 23, 2023

Uh oh!

kimdv commented May 23, 2023

Uh oh!

kimdv commented May 23, 2023

Uh oh!

kimdv commented May 23, 2023 • edited by ahoppen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahoppen commented May 23, 2023

Uh oh!

kimdv commented May 23, 2023

Uh oh!

kimdv commented May 23, 2023

Uh oh!

kimdv commented May 23, 2023

Uh oh!

Uh oh!

Uh oh!

kimdv commented May 24, 2023

Uh oh!

kimdv commented May 24, 2023

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

kimdv commented May 24, 2023

Uh oh!

kimdv commented May 25, 2023

Uh oh!

kimdv commented May 25, 2023

Uh oh!

kimdv commented May 29, 2023

Uh oh!

kimdv commented May 30, 2023

Uh oh!

kimdv commented May 30, 2023

Uh oh!

kimdv commented May 30, 2023

Uh oh!

Uh oh!

Remove `requiresLeadingNewline` from basic format #1690

Remove `requiresLeadingNewline` from basic format #1690

kimdv commented May 23, 2023 •

edited by ahoppen

Loading