Move regex literal parsing logic from SwiftCompilerSources to ASTGen #68401

DougGregor · 2023-09-08T18:25:34Z

ASTGen always builds with the host Swift compiler, without requiring bootstrapping, and is enabled in more places. Move the regex literal parsing logic there so it is enabled in more host environments, and makes use of CMake's Swift support. Enable all of the regex literal tests when ASTGen is built, to ensure everything is working.

Remove the "AST" and "Parse" Swift modules from SwiftCompilerSources, because they are no longer needed.

DougGregor · 2023-09-08T18:25:41Z

@swift-ci please smoke test

bnbarham

Seems like maybe we should move lib/ASTGen to something like swiftlib. Or maybe this all just becomes SwiftCompilerSources once we've moved that across as well.

Also, woo!

bnbarham · 2023-09-08T18:48:20Z

lib/ASTGen/Sources/ASTGen/Regex.swift

+//import AST
+//import Basic


Can remove now?

Yes, I forgot that I'd only commented them out

bnbarham · 2023-09-08T18:51:57Z

lib/ASTGen/Sources/ASTGen/Regex.swift

      let offset = str.utf8.distance(from: str.startIndex, to: errorLoc)
-      diagLoc = _diagLoc.advanced(by: offset)
+      diagLoc = BridgedSourceLoc(raw: _diagLoc.advanced(by: offset))


SourceLoc_advanced(bridgedDiagnosticBaseLoc, SwiftInt(offset))

(there's other places that assume raw is the address, but it would be good if we didn't add more)

Ah, good point

DougGregor · 2023-09-08T19:50:50Z

@swift-ci please clean smoke test Linux

DougGregor · 2023-09-08T19:50:58Z

@swift-ci please clean smoke test macOS

DougGregor · 2023-09-08T20:35:54Z

@swift-ci please smoke test

DougGregor · 2023-09-08T22:26:58Z

Seems like maybe we should move lib/ASTGen to something like swiftlib. Or maybe this all just becomes SwiftCompilerSources once we've moved that across as well.

I don't know if we want a swiftlib kind of thing, or a bunch of different libraries in various lib subdirectories. They're going to end up being intertwined with the C++ libraries.

bnbarham · 2023-09-08T22:51:46Z

Seems like maybe we should move lib/ASTGen to something like swiftlib. Or maybe this all just becomes SwiftCompilerSources once we've moved that across as well.

I don't know if we want a swiftlib kind of thing, or a bunch of different libraries in various lib subdirectories. They're going to end up being intertwined with the C++ libraries.

That's fine too, lib/ASTGen is just an odd mix now. SwiftCompilerSources and lib/ASTGen are also open-able as packages, we could do that in just lib as well but right now that would involve excluding all the other directories.

finagolfin · 2023-09-10T07:04:51Z

Thanks for doing this: I just applied this pull to the Sep. 2 trunk source snapshot and was able to build a Swift compiler natively on Android that supports regex literals again and Macros for the first time. Any plans to backport this patch to 5.9?

DougGregor · 2023-09-10T16:48:19Z

Thanks for doing this: I just applied this pull to the Sep. 2 trunk source snapshot and was able to build a Swift compiler natively on Android that supports regex literals again and Macros for the first time. Any plans to backport this patch to 5.9?

Fantastic! Yes, I'm going to address review comments and back port this patch to 5.9.

DougGregor · 2023-09-10T17:13:01Z

@swift-ci please smoke test

finagolfin · 2023-09-11T16:21:37Z

I went ahead and backported this pull to 5.9, slapped it up as a gist so you don't have to repeat that work. Haven't tried compiling it with the 5.9 branch yet, will do that next.

hamishknight

Nice!

hamishknight · 2023-09-14T09:04:55Z

tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp

+  initializeSwiftParseModules();
  initializeSwiftModules();


Would it be worth renaming initializeSwiftModules to something like initializeSwiftSILModules on the Swift side, and then defining initializeSwiftModules on the C side to call both that and initializeSwiftParseModules?

DougGregor · 2023-09-15T15:52:28Z

@swift-ci please test

DougGregor · 2023-09-15T15:53:09Z

@finagolfin we've got Windows CI updated and macros support landed there (#68334), so now it's time to get this tested and merged

DougGregor · 2023-09-15T15:53:26Z

@swift-ci please build toolchain Windows

finagolfin · 2023-09-15T16:00:37Z

@DougGregor, sounds good, I'm currently cross-compiling my 5.9 version of this pull for Android. It appears that portions of #66044 will also need to be backported to 5.9, or the 5.9 version of this pull will have to be adapted for that bridging pull not being in 5.9.

finagolfin · 2023-09-16T03:55:22Z

Linux CI failed when testing lldb:

Error output:
error: error while processing module import: error: <EXPR>:7:2: error: expected a macro identifier for a pound literal expression
#/Order from <(.*)>, type: (.*), count in dozen: ([0-9]+)/#

DougGregor · 2023-09-17T07:08:04Z

@swift-ci please test Linux

DougGregor · 2023-09-18T19:04:28Z

@swift-ci please test

DougGregor · 2023-09-19T15:30:27Z

@swift-ci please test

finagolfin · 2023-09-28T20:31:58Z

This will need to be updated for #68408.

ASTGen always builds with the host Swift compiler, without requiring bootstrapping, and is enabled in more places. Move the regex literal parsing logic there so it is enabled in more host environments, and makes use of CMake's Swift support. Enable all of the regex literal tests when ASTGen is built, to ensure everything is working. Remove the "AST" and "Parse" Swift modules from SwiftCompilerSources, because they are no longer needed.

DougGregor · 2023-09-29T23:59:38Z

Dusting this off now and resolving the conflicts.

DougGregor · 2023-09-30T00:01:03Z

swiftlang/llvm-project#7550

@swift-ci please test

DougGregor · 2023-09-30T02:06:34Z

swiftlang/llvm-project#7550

@swift-ci please test

bnbarham · 2023-09-30T02:11:15Z

lib/Parse/Lexer.cpp

@@ -35,11 +35,9 @@
 // Regex lexing delivered via libSwift.
 static RegexLiteralLexingFn regexLiteralLexingFn = nullptr;

-#if SWIFT_SWIFT_PARSER


It's not, it's just called SWIFT_BUILD_SWIFT_SYNTAX now. Was changed in the FetchContent PR. Do I regret that? Maybe a little.

We could totally get rid of it now, but I wouldn't for this PR personally.

ARGH. Force-pushed now that I realized this, about the same time you told me :)

Heh, sorry for not noticing it in here in the first place!

DougGregor · 2023-09-30T02:13:18Z

swiftlang/llvm-project#7550

@swift-ci please test

DougGregor · 2023-09-30T05:39:39Z

swiftlang/llvm-project#7550

@swift-ci please smoke test macOS

DougGregor · 2023-09-30T15:40:21Z

The macOS test never actually got started properly. Trying again...

DougGregor · 2023-09-30T15:40:27Z

swiftlang/llvm-project#7550

@swift-ci please test macOS

compnerd · 2023-09-30T21:00:50Z

Please test with following PRs:
swiftlang/llvm-project#7550
swiftlang/swift-installer-scripts#210

@swift-ci please build toolchain Windows platform

compnerd · 2023-10-01T01:34:15Z

Please test with following PRs:
swiftlang/llvm-project#7550
swiftlang/swift-installer-scripts#210

@swift-ci please build toolchain Windows platform

DougGregor · 2023-10-01T01:44:27Z

swiftlang/llvm-project#7550

@swift-ci please test

finagolfin · 2023-10-08T03:32:23Z

I have backported this pull to Swift 5.9 and am happy to report a working Swift 5.9 package with regex literals again on Android in the Termux app. I also backported #66033 and #66044 there, as this pull relies on those changes that aren't in the 5.9 branch.

I've also backported #68408 and swiftlang/swift-syntax#2173 and applied some changes so Swift Syntax can be cross-compiled (which I'll upstream next), so that Macros work natively on Android.

DougGregor · 2024-01-29T18:49:15Z

This was already done

DougGregor requested review from ahoppen, bnbarham, rintaro, hamishknight, hborla, xedin, CodaFi, zoecarver, slavapestov and eeckstein as code owners September 8, 2023 18:25

bnbarham reviewed Sep 8, 2023

View reviewed changes

DougGregor force-pushed the regex-literals-via-astgen branch from 118e8ef to 7159e72 Compare September 8, 2023 20:35

finagolfin mentioned this pull request Sep 12, 2023

[CMake] Replace early swift-syntax with FetchContent #68408

Merged

hamishknight approved these changes Sep 14, 2023

View reviewed changes

xedin removed their request for review September 15, 2023 14:56

DougGregor added 3 commits September 29, 2023 16:01

Address review feedback to avoid pointer arithmetic

6b4e706

Rename initializeSwiftModules to initializeSwiftSILModules().

3a3e467

DougGregor force-pushed the regex-literals-via-astgen branch from 5f3625d to 3a3e467 Compare September 29, 2023 23:58

DougGregor mentioned this pull request Sep 30, 2023

[Swift] Call both initializeSwiftParseModules and initializeSwiftSILModules swiftlang/llvm-project#7550

Open

bnbarham reviewed Sep 30, 2023

View reviewed changes

Use SWIFT_BUILD_SWIFT_SYNTAX instead of SWIFT_SWIFT_PARSER

773f511

DougGregor force-pushed the regex-literals-via-astgen branch from 4c4a16c to 773f511 Compare September 30, 2023 02:12

ahoppen removed their request for review October 2, 2023 21:12

DougGregor closed this Jan 29, 2024

DougGregor deleted the regex-literals-via-astgen branch January 29, 2024 18:49

		//import AST
		//import Basic

Move regex literal parsing logic from SwiftCompilerSources to ASTGen #68401

Move regex literal parsing logic from SwiftCompilerSources to ASTGen #68401

Conversation

DougGregor commented Sep 8, 2023

DougGregor commented Sep 8, 2023

bnbarham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DougGregor commented Sep 8, 2023

DougGregor commented Sep 8, 2023

DougGregor commented Sep 8, 2023

DougGregor commented Sep 8, 2023

bnbarham commented Sep 8, 2023

finagolfin commented Sep 10, 2023

DougGregor commented Sep 10, 2023

DougGregor commented Sep 10, 2023

finagolfin commented Sep 11, 2023

hamishknight left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DougGregor commented Sep 15, 2023

DougGregor commented Sep 15, 2023

DougGregor commented Sep 15, 2023

finagolfin commented Sep 15, 2023

finagolfin commented Sep 16, 2023

DougGregor commented Sep 17, 2023

DougGregor commented Sep 18, 2023

DougGregor commented Sep 19, 2023

finagolfin commented Sep 28, 2023

DougGregor commented Sep 29, 2023

DougGregor commented Sep 30, 2023

DougGregor commented Sep 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DougGregor commented Sep 30, 2023

DougGregor commented Sep 30, 2023

DougGregor commented Sep 30, 2023

DougGregor commented Sep 30, 2023

compnerd commented Sep 30, 2023

compnerd commented Oct 1, 2023

DougGregor commented Oct 1, 2023

finagolfin commented Oct 8, 2023

DougGregor commented Jan 29, 2024