[ASTGen] Generate AvailableAttr #79125

rintaro · 2025-02-04T00:59:49Z

Move AvailabilitySpec handling logic to AST, so they can be shared between libParse and ASTGen
Requestify -define-availability arguments parsing and parse them with SwiftParser according to the ParserASTGen feature flag
Implement AvailableAttr generation in ASTGen

rintaro · 2025-02-04T01:25:53Z

swiftlang/swift-syntax#2954
@swift-ci Please smoke test

rintaro · 2025-02-04T04:13:01Z

swiftlang/swift-syntax#2954
@swift-ci Please smoke test

rintaro · 2025-02-04T05:13:31Z

swiftlang/swift-syntax#2954
@swift-ci Please smoke test

include/swift/AST/ASTContext.h

rintaro · 2025-02-04T23:49:47Z

swiftlang/swift-syntax#2954
@swift-ci Please smoke test

* Move `AvailabilitySpec` handling logic to AST, so they can be shared between libParse and ASTGen * Requestify '-define-availability' arguments parsing and parse them with 'SwiftParser' according to the 'ParserASTGen' feature flag * Implement 'AvailableAttr' generation in ASTGen

rintaro · 2025-02-05T07:40:29Z

swiftlang/swift-syntax#2954
@swift-ci Please smoke test

hamishknight · 2025-02-05T15:35:16Z

include/swift/Basic/BasicBridging.h

+const void *_Nullable BridgedArrayRef_data(const BridgedArrayRef arr);

 SWIFT_NAME("getter:BridgedArrayRef.count(self:)")
-BRIDGED_INLINE SwiftInt BridgedArrayRef_count(BridgedArrayRef arr);
+BRIDGED_INLINE SwiftInt BridgedArrayRef_count(const BridgedArrayRef arr);
+
+SWIFT_NAME("getter:BridgedArrayRef.isEmpty(self:)")
+BRIDGED_INLINE bool BridgedArrayRef_isEmpty(const BridgedArrayRef arr);


The consts on the parameters don't do anything, right?

hamishknight · 2025-02-05T15:41:55Z

include/swift/Basic/BasicBridgingImpl.h

+const void *_Nullable BridgedArrayRef_data(const BridgedArrayRef arr) {
  return arr.Data;
 }

-SwiftInt BridgedArrayRef_count(BridgedArrayRef arr) {
+SwiftInt BridgedArrayRef_count(const BridgedArrayRef arr) {
  return static_cast<SwiftInt>(arr.Length);
 }

+bool BridgedArrayRef_isEmpty(const BridgedArrayRef arr) {
+  return arr.Length == 0;
+}


Similar comment, although these do prevent mutating the variable itself, but that's not too helpful

hamishknight · 2025-02-05T15:58:11Z

lib/ASTGen/Sources/ASTGen/Availability.swift

@@ -0,0 +1,426 @@
+//===--- Attrs.swift ------------------------------------------------------===//


Suggested change

//===--- Attrs.swift ------------------------------------------------------===//

//===--- Availability.swift -----------------------------------------------===//

hamishknight · 2025-02-05T16:00:15Z

lib/ASTGen/Sources/ASTGen/Availability.swift

+    let atLoc = self.generateSourceLoc(node.atSign)
+    let range = self.generateAttrSourceRange(node)
+    let specs = self.generateAvailabilitySpecList(
+      args: node.arguments!.cast(AvailabilityArgumentListSyntax.self),


Worth passing through the args so you don't have to do this again?

hamishknight · 2025-02-05T16:00:39Z

lib/ASTGen/Sources/ASTGen/Availability.swift

+  }
+
+  func generateAvailableAttrExtended(attribute node: AttributeSyntax) -> BridgedAvailableAttr? {
+    var args = node.arguments!.cast(AvailabilityArgumentListSyntax.self)[...]


hamishknight · 2025-02-05T16:01:50Z

lib/ASTGen/Sources/ASTGen/Availability.swift

+    var renamed: BridgedStringRef? = nil
+
+    func generateVersion(arg: AvailabilityLabeledArgumentSyntax, into target: inout VersionAndRange?) {
+      guard let versionSytnax = arg.value.as(VersionTupleSyntax.self) else {


Suggested change

guard let versionSytnax = arg.value.as(VersionTupleSyntax.self) else {

guard let versionSyntax = arg.value.as(VersionTupleSyntax.self) else {

hamishknight · 2025-02-05T16:09:27Z

lib/ASTGen/Sources/ASTGen/Bridge.swift

+
+extension BridgedArrayRef {
+  public func withElements<T, R>(ofType ty: T.Type, _ c: (UnsafeBufferPointer<T>) -> R) -> R {
+    let start = data?.bindMemory(to: ty, capacity: count)


I feel like this should probably either use withMemoryRebound(to:) or assumingMemoryBound(to:)

Indeed, I'd go with assumingMemoryBound. I copied this from SwiftCompilerSources, so I will change both in followups.

hamishknight · 2025-02-05T16:14:01Z

include/swift/AST/ParseRequests.h

+/// Parse the '-define-availability' arguments.
+class AvailabilityMacroArgumentsRequest
+    : public SimpleRequest<AvailabilityMacroArgumentsRequest,
+                           AvailabilityMacroMap *(ASTContext *),


I think this should hand back a const pointer

Suggested change

AvailabilityMacroMap *(ASTContext *),

const AvailabilityMacroMap *(ASTContext *),

hamishknight · 2025-02-05T16:17:29Z

lib/AST/Bridging/ASTContextBridging.cpp

+
+BridgedAvailabilityMacroMap
+BridgedASTContext_getAvailabilityMacroMap(BridgedASTContext cContext) {
+  return const_cast<AvailabilityMacroMap *>(


Would be nice if BridgedAvailabilityMacroMap could store a const pointer, maybe we should define it manually instead of using AST_BRIDGING_WRAPPER? Or maybe we could add a const variant of that macro

rintaro · 2025-02-05T17:40:56Z

Thanks @hamishknight ! I will address your comments in followups

rintaro force-pushed the astgen-available-attr branch 3 times, most recently from 9333a24 to b8c8b20 Compare February 4, 2025 20:01

rintaro marked this pull request as ready for review February 4, 2025 21:21

rintaro requested review from ahoppen, bnbarham, CodaFi, hamishknight, DougGregor, tshortli, hborla, slavapestov and xedin as code owners February 4, 2025 21:21

rintaro commented Feb 4, 2025

View reviewed changes

include/swift/AST/ASTContext.h Outdated Show resolved Hide resolved

rintaro force-pushed the astgen-available-attr branch from b8c8b20 to 6a0fda6 Compare February 4, 2025 21:51

tshortli approved these changes Feb 5, 2025

View reviewed changes

rintaro force-pushed the astgen-available-attr branch from 6a0fda6 to cf62f3d Compare February 5, 2025 03:38

rintaro force-pushed the astgen-available-attr branch from cf62f3d to df2ada3 Compare February 5, 2025 07:40

hamishknight approved these changes Feb 5, 2025

View reviewed changes

rintaro merged commit a619daf into swiftlang:main Feb 5, 2025
3 checks passed

rintaro mentioned this pull request Feb 5, 2025

Parser: Move availability macro definition cache to a request #76829

Open

		@@ -0,0 +1,426 @@
		//===--- Attrs.swift ------------------------------------------------------===//

	//===--- Attrs.swift ------------------------------------------------------===//
	//===--- Availability.swift -----------------------------------------------===//

	guard let versionSytnax = arg.value.as(VersionTupleSyntax.self) else {
	guard let versionSyntax = arg.value.as(VersionTupleSyntax.self) else {

	AvailabilityMacroMap (ASTContext ),
	const AvailabilityMacroMap (ASTContext ),

[ASTGen] Generate AvailableAttr #79125

[ASTGen] Generate AvailableAttr #79125

Uh oh!

Conversation

rintaro commented Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rintaro commented Feb 4, 2025

Uh oh!

rintaro commented Feb 4, 2025

Uh oh!

rintaro commented Feb 4, 2025

Uh oh!

Uh oh!

rintaro commented Feb 4, 2025

Uh oh!

rintaro commented Feb 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hamishknight Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro commented Feb 5, 2025

Uh oh!

Uh oh!

Uh oh!

rintaro commented Feb 4, 2025 •

edited

Loading

hamishknight Feb 5, 2025 •

edited

Loading