From d494c046170d9dca4e0b7474477ab2c62bf2caa0 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 5 Sep 2024 11:37:52 -0400 Subject: [PATCH 1/4] Attempt to work around compiler regression causing CI failure. This PR attempts to work around the compiler regression reported in rdar://135346598 that is currently causing our macOS CI jobs to fail. --- .../Support/AttributeDiscovery.swift | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/Sources/TestingMacros/Support/AttributeDiscovery.swift b/Sources/TestingMacros/Support/AttributeDiscovery.swift index 5f98f4fc8..59f405d9f 100644 --- a/Sources/TestingMacros/Support/AttributeDiscovery.swift +++ b/Sources/TestingMacros/Support/AttributeDiscovery.swift @@ -17,19 +17,20 @@ import SwiftSyntaxMacros /// If the developer specified Self.something as an argument to the `@Test` or /// `@Suite` attribute, we will currently incorrectly infer Self as equalling /// the `__TestContainer` type we emit rather than the type containing the -/// test. This class strips off `Self.` wherever that occurs. +/// test. This class strips off `Self.` wherever that occurs and, if possible, +/// replaces it with the actual containing type name. /// /// Note that this operation is technically incorrect if a subexpression of the /// attribute declares a type and refers to it with `Self`. We accept this /// constraint as it is unlikely to pose real-world issues and is generally /// solvable by using an explicit type name instead of `Self`. -/// -/// This class should instead replace `Self` with the name of the containing -/// type when rdar://105470382 is resolved. private final class _SelfRemover: SyntaxRewriter where C: MacroExpansionContext { /// The macro context in which the expression is being parsed. let context: C + /// The type with which to replace `Self`. + let typeExpr: TypeExprSyntax? + /// Initialize an instance of this class. /// /// - Parameters: @@ -37,18 +38,22 @@ private final class _SelfRemover: SyntaxRewriter where C: MacroExpansionConte /// - viewMode: The view mode to use when walking the syntax tree. init(in context: C) { self.context = context + self.typeExpr = context.typeOfLexicalContext.map { TypeExprSyntax(type: $0) } } override func visit(_ node: MemberAccessExprSyntax) -> ExprSyntax { if let base = node.base?.as(DeclReferenceExprSyntax.self) { if base.baseName.tokenKind == .keyword(.Self) { - // We cannot currently correctly convert Self.self into the expected - // type name, but once rdar://105470382 is resolved we can replace the - // base expression with the typename here (at which point Self.self - // ceases to be an interesting case anyway.) + if let typeExpr { + // Replace Self with the actual type name. + return ExprSyntax(node.with(\.base, ExprSyntax(typeExpr))) + } + // We don't know the enclosing type name, so just strip Self and hope + // the compiler is happy with the leading dot syntax. return ExprSyntax(node.declName) } } else if let base = node.base?.as(MemberAccessExprSyntax.self) { + // Recursively walk into the base expression. return ExprSyntax(node.with(\.base, visit(base))) } return ExprSyntax(node) From 67f4b3b5de9d92611b433d85345ba7fd9e0847e4 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 5 Sep 2024 11:42:43 -0400 Subject: [PATCH 2/4] Revert "Attempt to work around compiler regression causing CI failure." This reverts commit d494c046170d9dca4e0b7474477ab2c62bf2caa0. --- .../Support/AttributeDiscovery.swift | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/Sources/TestingMacros/Support/AttributeDiscovery.swift b/Sources/TestingMacros/Support/AttributeDiscovery.swift index 59f405d9f..5f98f4fc8 100644 --- a/Sources/TestingMacros/Support/AttributeDiscovery.swift +++ b/Sources/TestingMacros/Support/AttributeDiscovery.swift @@ -17,20 +17,19 @@ import SwiftSyntaxMacros /// If the developer specified Self.something as an argument to the `@Test` or /// `@Suite` attribute, we will currently incorrectly infer Self as equalling /// the `__TestContainer` type we emit rather than the type containing the -/// test. This class strips off `Self.` wherever that occurs and, if possible, -/// replaces it with the actual containing type name. +/// test. This class strips off `Self.` wherever that occurs. /// /// Note that this operation is technically incorrect if a subexpression of the /// attribute declares a type and refers to it with `Self`. We accept this /// constraint as it is unlikely to pose real-world issues and is generally /// solvable by using an explicit type name instead of `Self`. +/// +/// This class should instead replace `Self` with the name of the containing +/// type when rdar://105470382 is resolved. private final class _SelfRemover: SyntaxRewriter where C: MacroExpansionContext { /// The macro context in which the expression is being parsed. let context: C - /// The type with which to replace `Self`. - let typeExpr: TypeExprSyntax? - /// Initialize an instance of this class. /// /// - Parameters: @@ -38,22 +37,18 @@ private final class _SelfRemover: SyntaxRewriter where C: MacroExpansionConte /// - viewMode: The view mode to use when walking the syntax tree. init(in context: C) { self.context = context - self.typeExpr = context.typeOfLexicalContext.map { TypeExprSyntax(type: $0) } } override func visit(_ node: MemberAccessExprSyntax) -> ExprSyntax { if let base = node.base?.as(DeclReferenceExprSyntax.self) { if base.baseName.tokenKind == .keyword(.Self) { - if let typeExpr { - // Replace Self with the actual type name. - return ExprSyntax(node.with(\.base, ExprSyntax(typeExpr))) - } - // We don't know the enclosing type name, so just strip Self and hope - // the compiler is happy with the leading dot syntax. + // We cannot currently correctly convert Self.self into the expected + // type name, but once rdar://105470382 is resolved we can replace the + // base expression with the typename here (at which point Self.self + // ceases to be an interesting case anyway.) return ExprSyntax(node.declName) } } else if let base = node.base?.as(MemberAccessExprSyntax.self) { - // Recursively walk into the base expression. return ExprSyntax(node.with(\.base, visit(base))) } return ExprSyntax(node) From 0c8e8734c3e1100f33612c17d04c247725726bc0 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 5 Sep 2024 11:43:26 -0400 Subject: [PATCH 3/4] Guess that didn't work, just hide the offending fixture instead --- Tests/TestingTests/MiscellaneousTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift index b9f274c8e..376eb587c 100644 --- a/Tests/TestingTests/MiscellaneousTests.swift +++ b/Tests/TestingTests/MiscellaneousTests.swift @@ -166,8 +166,10 @@ struct TestsWithStaticMemberAccessBySelfKeyword { @Test(.hidden, arguments: Self.x) func f(i: Int) {} +#if FIXED_135346598 @Test(.hidden, arguments: Self.f(max: 100)) func g(i: Int) {} +#endif @Test(.hidden, arguments: [Self.f(max:)]) func h(i: @Sendable (Int) -> Range) {} From 55b26efadbee33fc039780cb81733f2ef7368b92 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 5 Sep 2024 11:46:29 -0400 Subject: [PATCH 4/4] More fixtures trigger the issue --- Tests/TestingTests/MiscellaneousTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift index 376eb587c..bb0d4b7f4 100644 --- a/Tests/TestingTests/MiscellaneousTests.swift +++ b/Tests/TestingTests/MiscellaneousTests.swift @@ -169,7 +169,6 @@ struct TestsWithStaticMemberAccessBySelfKeyword { #if FIXED_135346598 @Test(.hidden, arguments: Self.f(max: 100)) func g(i: Int) {} -#endif @Test(.hidden, arguments: [Self.f(max:)]) func h(i: @Sendable (Int) -> Range) {} @@ -180,6 +179,7 @@ struct TestsWithStaticMemberAccessBySelfKeyword { @Test(.hidden, arguments: [Box(rawValue: Self.f(max:))]) func j(i: Box<@Sendable (Int) -> Range>) {} +#endif struct Nested { static let x = 0 ..< 100