Apply Doug's review comments

zoecarver · zoecarver · commit 0631f1baa640 · 2022-09-07T20:05:17.000-07:00
diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def
@@ -126,6 +126,9 @@ EXPERIMENTAL_FEATURE(OpaqueTypeErasure)
 /// wrapped type.
 EXPERIMENTAL_FEATURE(TypeWrappers)
 
+/// Parse using the Swift (swift-syntax) parser and use ASTGen.
+EXPERIMENTAL_FEATURE(SwiftParser)
+
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE
 #undef SUPPRESSIBLE_LANGUAGE_FEATURE
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h
@@ -274,8 +274,6 @@ namespace swift {
     /// disabled because it is not complete.
     bool EnableCXXInterop = false;
 
-    bool EnableSwiftParser = false;
-
     /// Imports getters and setters as computed properties.
     bool CxxInteropGettersSettersAsProperties = false;
 
diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td
@@ -621,11 +621,6 @@ def enable_experimental_cxx_interop :
   Flags<[FrontendOption, HelpHidden, ModuleInterfaceOption]>,
   HelpText<"Enable experimental C++ interop code generation and config directives">;
 
-def enable_experimental_swift_parser :
-  Flag<["-"], "enable-experimental-swift-parser">,
-  Flags<[FrontendOption, HelpHidden, ModuleInterfaceOption]>,
-  HelpText<"Enable the new and expiremental Swift parser + AST generation.">;
-
 def experimental_cxx_stdlib :
   Separate<["-"], "experimental-cxx-stdlib">,
   Flags<[HelpHidden]>,
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
@@ -3083,6 +3083,10 @@ static bool usesFeatureAdditiveArithmeticDerivedConformances(Decl *decl) {
   return false;
 }
 
+static bool usesFeatureSwiftParser(Decl *decl) {
+  return false;
+}
+
 static void
 suppressingFeatureNoAsyncAvailability(PrintOptions &options,
                                       llvm::function_ref<void()> action) {
diff --git a/lib/AST/CMakeLists.txt b/lib/AST/CMakeLists.txt
@@ -155,7 +155,7 @@ endif()
 
 target_link_libraries(swiftAST
   PUBLIC swiftBasic
-  PRIVATE legacySwiftSyntax)
+  PRIVATE swiftSyntax)
 if(SWIFT_BUILD_ONLY_SYNTAXPARSERLIB)
   # Remove dependencies from clangBasic to avoid bringing along some llvm
   # libraries that we don't need to be building.
diff --git a/lib/ASTGen/CMakeLists.txt b/lib/ASTGen/CMakeLists.txt
@@ -1,27 +1,29 @@
-add_subdirectory(${SWIFT_SOURCE_DIR}/../swift-syntax ${CMAKE_CURRENT_BINARY_DIR}/swift-syntax)
+# TODO: why do we need these on Linux?
+set(CMAKE_Swift_ARCHIVE_CREATE "<CMAKE_AR> crs <TARGET> <OBJECTS>")
+set(CMAKE_Swift_ARCHIVE_FINISH "")
 
-add_library(ASTGen
-        ASTGen.swift)
+if (SWIFT_SWIFT_PARSER)
+    add_library(ASTGen
+            ASTGen.swift)
 
-target_compile_options(ASTGen PUBLIC
-        "-emit-module-interface")
+    target_link_libraries(ASTGen PUBLIC
+            SwiftSyntax
+            SwiftParser
+            SwiftDiagnostics)
+
+    target_link_directories(ASTGen PUBLIC
+            "${SWIFT_PATH_TO_EARLYSWIFTSYNTAX_BUILD_DIR}/lib")
 
-target_link_libraries(ASTGen PUBLIC
-  SwiftSyntax
-  SwiftParser
-  SwiftDiagnostics)
-  
-target_include_directories(SwiftParser PUBLIC
-  "${CMAKE_CURRENT_BINARY_DIR}/../Parser/swift-syntax/SwiftSyntax"
-  "${CMAKE_CURRENT_BINARY_DIR}/../Parser/swift-syntax/SwiftDiagnostics"
-  "${CMAKE_CURRENT_BINARY_DIR}/../Parser/swift-syntax/SwiftParser")
+    target_include_directories(ASTGen PUBLIC
+            "${SWIFT_PATH_TO_EARLYSWIFTSYNTAX_BUILD_DIR}/swift")
+else()
+    add_library(ASTGen
+            DummyASTGen.swift)
+endif()
 
-set_property(GLOBAL APPEND PROPERTY SWIFTSYNTAX_EXPORTS ASTGen)
+set_target_properties(ASTGen PROPERTIES LINKER_LANGUAGE Swift)
 
-install(TARGETS ASTGen
-  EXPORT SwiftSyntaxTargets
-  ARCHIVE DESTINATION lib
-  LIBRARY DESTINATION lib
-  RUNTIME DESTINATION bin)
+target_compile_options(ASTGen PUBLIC
+        "-emit-module-interface")
 
 add_dependencies(ASTGen swiftAST)
diff --git a/lib/ASTGen/DummyASTGen.swift b/lib/ASTGen/DummyASTGen.swift
@@ -0,0 +1,9 @@
+@_cdecl("parseTopLevelSwift")
+public func parseTopLevelSwift(
+    buffer: UnsafePointer<CChar>, declContext: UnsafeMutableRawPointer,
+    ctx: UnsafeMutableRawPointer,
+    outputContext: UnsafeMutableRawPointer,
+    callback: @convention(c) (UnsafeMutableRawPointer, UnsafeMutableRawPointer) -> Void
+) {
+  fatalError("Please run build script with --early-swiftsyntax to use ASTGen.")
+}
diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
@@ -16,7 +16,7 @@ list(APPEND LLVM_COMMON_DEPENDS swift-parse-syntax-generated-headers)
 
 add_subdirectory(APIDigester)
 add_subdirectory(AST)
-add_subdirectory(ASTGen) # Must come before Parse
+add_subdirectory(ASTGen)
 add_subdirectory(ASTSectionImporter)
 add_subdirectory(Basic)
 add_subdirectory(ConstExtract)
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -842,7 +842,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   }
 
   Opts.EnableCXXInterop |= Args.hasArg(OPT_enable_experimental_cxx_interop);
-  Opts.EnableSwiftParser |= Args.hasArg(OPT_enable_experimental_swift_parser);
   Opts.EnableObjCInterop =
       Args.hasFlag(OPT_enable_objc_interop, OPT_disable_objc_interop,
                    Target.isOSDarwin());
diff --git a/lib/Migrator/CMakeLists.txt b/lib/Migrator/CMakeLists.txt
@@ -57,7 +57,7 @@ add_swift_host_library(swiftMigrator STATIC
   RewriteBufferEditsReceiver.cpp)
 target_link_libraries(swiftMigrator PRIVATE
   swiftIDE
-  legacySwiftSyntax)
+  swiftSyntax)
 
 add_dependencies(swiftMigrator
   "symlink_migrator_data")
diff --git a/lib/Parse/CMakeLists.txt b/lib/Parse/CMakeLists.txt
@@ -31,7 +31,7 @@ _swift_gyb_target_sources(swiftParse PRIVATE
     ParsedSyntaxRecorder.cpp.gyb)
 target_link_libraries(swiftParse PRIVATE
   swiftAST
-  legacySwiftSyntax
+  swiftSyntax
   swiftSyntaxParse
 )
 
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -184,7 +184,7 @@ void Parser::parseTopLevel(SmallVectorImpl<Decl *> &decls) {
   StringRef contents =
       SourceMgr.extractText(SourceMgr.getRangeForBuffer(L->getBufferID()));
 
-  if (Context.LangOpts.EnableSwiftParser) {
+  if (Context.LangOpts.hasFeature(Feature::SwiftParser)) {
     parseTopLevelSwift(contents.data(), CurDeclContext, &Context, &decls, appendToVector);
 
     for (auto decl : decls) {
diff --git a/lib/Syntax/CMakeLists.txt b/lib/Syntax/CMakeLists.txt
@@ -4,19 +4,19 @@ else()
   set(SWIFT_GYB_FLAGS --line-directive "\'#line" "%(line)d" "\"%(file)s\"\'")
 endif()
 
-add_swift_host_library(legacySwiftSyntax STATIC
+add_swift_host_library(swiftSyntax STATIC
   AbsoluteRawSyntax.cpp
   RawSyntax.cpp
   Syntax.cpp
   SyntaxData.cpp
   UnknownSyntax.cpp)
-_swift_gyb_target_sources(legacySwiftSyntax PRIVATE
+_swift_gyb_target_sources(swiftSyntax PRIVATE
     SyntaxNodes.cpp.gyb
     SyntaxBuilders.cpp.gyb
     SyntaxKind.cpp.gyb
     SyntaxFactory.cpp.gyb
     SyntaxVisitor.cpp.gyb
     Trivia.cpp.gyb
     SyntaxSerialization.cpp.gyb)
-target_link_libraries(legacySwiftSyntax PRIVATE
+target_link_libraries(swiftSyntax PRIVATE
   swiftParse)
diff --git a/lib/SyntaxParse/CMakeLists.txt b/lib/SyntaxParse/CMakeLists.txt
@@ -3,6 +3,6 @@ add_swift_host_library(swiftSyntaxParse STATIC
   SyntaxTreeCreator.cpp)
 target_link_libraries(swiftSyntaxParse PRIVATE
   swiftParse
-  legacySwiftSyntax)
+  swiftSyntax)
 
 set_swift_llvm_is_available(swiftSyntaxParse)
diff --git a/tools/SourceKit/lib/SwiftLang/CMakeLists.txt b/tools/SourceKit/lib/SwiftLang/CMakeLists.txt
@@ -41,7 +41,7 @@ target_link_libraries(SourceKitSwiftLang PRIVATE
   swiftSema
   swiftBasic
   swiftSerialization
-  legacySwiftSyntax
+  swiftSyntax
   swiftOption
   swiftSymbolGraphGen
   libcmark-gfm_static
diff --git a/tools/driver/CMakeLists.txt b/tools/driver/CMakeLists.txt
@@ -14,6 +14,8 @@ if(${BOOTSTRAPPING_MODE} MATCHES "BOOTSTRAPPING.*")
   )
   target_link_libraries(swift-frontend-bootstrapping0
                         PRIVATE
+                          ASTGen
+                          swiftCore
                           swiftDriverTool
                           swiftCompilerStub)
 
@@ -33,6 +35,8 @@ if(${BOOTSTRAPPING_MODE} MATCHES "BOOTSTRAPPING.*")
   )
   target_link_libraries(swift-frontend-bootstrapping1
                         PRIVATE
+                          ASTGen
+                          swiftCore
                           swiftDriverTool
                           swiftCompilerModules-bootstrapping1)
 
@@ -50,11 +54,12 @@ add_swift_host_tool(swift-frontend
 target_link_libraries(swift-frontend
                       PUBLIC
                         ASTGen
-                        SwiftParser
-                        SwiftSyntax
+                        swiftCore
                         swiftDriverTool
                         swiftCompilerModules)
 
+_add_swift_runtime_link_flags(swift-frontend "../../lib" "")
+
 # Create a `swift-driver` executable adjacent to the `swift-frontend` executable
 # to ensure that `swiftc` forwards to the standalone driver when invoked.
 swift_create_early_driver_copies(swift-frontend)
diff --git a/tools/swift-syntax-test/CMakeLists.txt b/tools/swift-syntax-test/CMakeLists.txt
@@ -11,5 +11,5 @@ target_link_libraries(swift-syntax-test
                         swiftDriver
                         swiftFrontend
                         swiftSema
-                        legacySwiftSyntax
+                        swiftSyntax
                         swiftCompilerModules_SwiftSyntax)
diff --git a/unittests/Syntax/CMakeLists.txt b/unittests/Syntax/CMakeLists.txt
@@ -10,4 +10,4 @@ add_swift_unittest(SwiftSyntaxTests
 
 target_link_libraries(SwiftSyntaxTests
   PRIVATE
-  legacySwiftSyntax)
+  swiftSyntax)
diff --git a/unittests/SyntaxParser/CMakeLists.txt b/unittests/SyntaxParser/CMakeLists.txt
@@ -11,7 +11,7 @@ endif()
 target_link_libraries(SwiftSyntaxParserTests
   PRIVATE
   libSwiftSyntaxParser
-  legacySwiftSyntax)
+  swiftSyntax)
 
 if(CMAKE_SYSTEM_NAME STREQUAL Darwin)
   set_target_properties(SwiftSyntaxParserTests PROPERTIES

Original file line number	Diff line number	Diff line change
`@@ -842,7 +842,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,`
`842`	`842`	`}`
`843`	`843`
`844`	`844`	`Opts.EnableCXXInterop \|= Args.hasArg(OPT_enable_experimental_cxx_interop);`
`845`		`- Opts.EnableSwiftParser \|= Args.hasArg(OPT_enable_experimental_swift_parser);`
`846`	`845`	`Opts.EnableObjCInterop =`
`847`	`846`	`Args.hasFlag(OPT_enable_objc_interop, OPT_disable_objc_interop,`
`848`	`847`	`Target.isOSDarwin());`