From d7ad95e8b3986e56d867a3952e4490362c2f12fd Mon Sep 17 00:00:00 2001 From: Rauhul Varma Date: Thu, 9 Nov 2023 11:17:09 -0800 Subject: [PATCH] Lift restrictions on computed properties Updates the RegisterBank and Register macros to ignore computed properties in structs they are attached to. Updates tests to cover this usecase and adds targeted tests for `VariableDeclSyntax.isComputedProperty` added in this commit. Fixes #29 --- .../MMIOMacros/Macros/RegisterBankMacro.swift | 13 +- Sources/MMIOMacros/Macros/RegisterMacro.swift | 13 +- .../VariableDeclSyntax.swift | 30 +++++ .../RegisterBankAndOffsetMacroTests.swift | 2 +- .../Macros/RegisterBankMacroTests.swift | 29 ++++- .../Macros/RegisterMacroTests.swift | 111 +++++++++++++----- .../VariableDeclSyntaxTests.swift | 53 +++++++++ 7 files changed, 208 insertions(+), 43 deletions(-) diff --git a/Sources/MMIOMacros/Macros/RegisterBankMacro.swift b/Sources/MMIOMacros/Macros/RegisterBankMacro.swift index 8f45cc7e..0827f182 100644 --- a/Sources/MMIOMacros/Macros/RegisterBankMacro.swift +++ b/Sources/MMIOMacros/Macros/RegisterBankMacro.swift @@ -46,10 +46,15 @@ extension RegisterBankMacro: MMIOMemberMacro { // Walk all the members of the struct. var error = false for member in structDecl.memberBlock.members { - // Ignore non-variable declaration. - guard let variableDecl = member.decl.as(VariableDeclSyntax.self) else { + guard + // Ignore non-variable declarations. + let variableDecl = member.decl.as(VariableDeclSyntax.self), + // Ignore non-stored properties. + !variableDecl.isComputedProperty + else { continue } + // Each variable declaration must be annotated with the // RegisterBankOffsetMacro. Further syntactic checking will be performed // by that macro. @@ -64,10 +69,10 @@ extension RegisterBankMacro: MMIOMemberMacro { // Retrieve the access level of the struct, so we can use the same // access level for the unsafeAddress property and initializer. - let acl = structDecl.accessLevel + let acl = structDecl.accessLevel?.trimmed return [ - "\(acl)private(set) var unsafeAddress: UInt", + "\(acl) let unsafeAddress: UInt", """ #if FEATURE_INTERPOSABLE var interposer: (any MMIOInterposer)? diff --git a/Sources/MMIOMacros/Macros/RegisterMacro.swift b/Sources/MMIOMacros/Macros/RegisterMacro.swift index 37dcb43c..2e2d4b12 100644 --- a/Sources/MMIOMacros/Macros/RegisterMacro.swift +++ b/Sources/MMIOMacros/Macros/RegisterMacro.swift @@ -50,7 +50,7 @@ extension RegisterMacro: MMIOMemberMacro { let declaration = declaration as! DeclSyntaxProtocol // Can only applied to structs. let structDecl = try declaration.requireAs(StructDeclSyntax.self, context) - let accessLevel = structDecl.accessLevel + let accessLevel = structDecl.accessLevel?.trimmed let bitWidth = self.bitWidth.value // Walk all the members of the struct. @@ -58,15 +58,18 @@ extension RegisterMacro: MMIOMemberMacro { var isSymmetric = true var bitFields = [BitFieldDescription]() for member in structDecl.memberBlock.members { - // Each member must be a variable declaration. - guard let variableDecl = member.decl.as(VariableDeclSyntax.self) else { - _ = context.error(at: member.decl, message: .onlyMemberVarDecls()) - error = true + guard + // Ignore non-variable declarations. + let variableDecl = member.decl.as(VariableDeclSyntax.self), + // Ignore non-stored properties. + !variableDecl.isComputedProperty + else { continue } guard // Each declaration must be annotated with exactly one bitField macro. + // Further syntactic checking will be performed by that macro. let value = try? variableDecl.requireMacro(bitFieldMacros, context), // This will always succeed. let macroType = value.macroType as? any (BitFieldMacro.Type), diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/VariableDeclSyntax.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/VariableDeclSyntax.swift index d35f01a5..8d9608ca 100644 --- a/Sources/MMIOMacros/SwiftSyntaxExtensions/VariableDeclSyntax.swift +++ b/Sources/MMIOMacros/SwiftSyntaxExtensions/VariableDeclSyntax.swift @@ -51,6 +51,36 @@ extension VariableDeclSyntax { } } +extension VariableDeclSyntax { + var isComputedProperty: Bool { + guard + self.bindings.count == 1, + let binding = self.bindings.first + else { + // Computed properties cannot have multiple bindings. + return false + } + + // Computed properties must have an accessor block + guard let accessorBlock = binding.accessorBlock else { return false } + + switch accessorBlock.accessors { + case .accessors(let accessors): + for accessor in accessors { + switch accessor.accessorSpecifier.tokenKind { + case .keyword(.willSet), .keyword(.didSet): + return false + default: + return true + } + } + return false + case .getter: + return true + } + } +} + extension ErrorDiagnostic { static func expectedBindingSpecifier(_ node: Keyword) -> Self { .init("'\(Macro.signature)' can only be applied to '\(node)' bindings") diff --git a/Tests/MMIOMacrosTests/Macros/RegisterBankAndOffsetMacroTests.swift b/Tests/MMIOMacrosTests/Macros/RegisterBankAndOffsetMacroTests.swift index 31402af9..06a9c09c 100644 --- a/Tests/MMIOMacrosTests/Macros/RegisterBankAndOffsetMacroTests.swift +++ b/Tests/MMIOMacrosTests/Macros/RegisterBankAndOffsetMacroTests.swift @@ -55,7 +55,7 @@ final class RegisterBankAndOffsetMacroTests: XCTestCase { } } - private (set) var unsafeAddress: UInt + let unsafeAddress: UInt #if FEATURE_INTERPOSABLE var interposer: (any MMIOInterposer)? diff --git a/Tests/MMIOMacrosTests/Macros/RegisterBankMacroTests.swift b/Tests/MMIOMacrosTests/Macros/RegisterBankMacroTests.swift index 738c25f8..931c3f45 100644 --- a/Tests/MMIOMacrosTests/Macros/RegisterBankMacroTests.swift +++ b/Tests/MMIOMacrosTests/Macros/RegisterBankMacroTests.swift @@ -74,19 +74,21 @@ final class RegisterBankMacroTests: XCTestCase { macros: Self.macros) } - func test_members_varDeclsAreAnnotated() { + func test_members_storedVarDeclsAreAnnotated() { assertMacroExpansion( """ @RegisterBank struct S { var v1: Int @OtherAttribute var v2: Int + var v3: Int { willSet {} } } """, expandedSource: """ struct S { var v1: Int @OtherAttribute var v2: Int + var v3: Int { willSet {} } } """, diagnostics: [ @@ -112,26 +114,47 @@ final class RegisterBankMacroTests: XCTestCase { fixIts: [ .init(message: "Insert '@RegisterBank(offset:)' macro") ]), + .init( + message: + ErrorDiagnostic + .expectedMemberAnnotatedWithMacro([RegisterBankOffsetMacro.self]) + .message, + line: 5, + column: 3, + highlight: "var v3: Int { willSet {} }", + fixIts: [ + .init(message: "Insert '@RegisterBank(offset:)' macro") + ]), ], macros: Self.macros, indentationWidth: Self.indentationWidth) } - func test_members_nonVarDeclIsOk() { + func test_members_nonStoredVarDeclsAreOk() { assertMacroExpansion( """ @RegisterBank struct S { func f() {} class C {} + var v: Void {} + var v: Void { get {} } + var v: Void { set {} } + var v: Void { _read {} } + var v: Void { _modify {} } } """, expandedSource: """ struct S { func f() {} class C {} + var v: Void {} + var v: Void { get {} } + var v: Void { set {} } + var v: Void { _read {} } + var v: Void { _modify {} } - private (set) var unsafeAddress: UInt + let unsafeAddress: UInt #if FEATURE_INTERPOSABLE var interposer: (any MMIOInterposer)? diff --git a/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift b/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift index 67730aeb..682854ac 100644 --- a/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift +++ b/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift @@ -66,56 +66,35 @@ final class RegisterMacroTests: XCTestCase { indentationWidth: Self.indentationWidth) } - func test_members_onlyVarDecls() { + func test_decl_onlyStruct_broken() { assertMacroExpansion( """ - @Register(bitWidth: 0x8) - struct S { - func f() {} - class C {} - } + @Register(bitWidth: 0x8) var v: Int """, expandedSource: """ - struct S { - func f() {} - class C {} - } - - extension S: RegisterValue { - } + var v: Int """, diagnostics: [ - .init( - message: ErrorDiagnostic.onlyMemberVarDecls().message, - line: 3, - column: 3, - // FIXME: Improve this highlight - highlight: "func f() {}"), - .init( - message: ErrorDiagnostic.onlyMemberVarDecls().message, - line: 4, - column: 3, - // FIXME: Improve this highlight - highlight: "class C {}"), - + // FIXME: https://github.com/apple/swift-syntax/issues/2206 ], - macros: Self.macros, - indentationWidth: Self.indentationWidth) + macros: Self.macros) } - func test_members_varDeclsAreAnnotated() { + func test_members_storedVarDeclsAreAnnotated() { assertMacroExpansion( """ @Register(bitWidth: 0x8) struct S { var v1: Int @OtherAttribute var v2: Int + var v3: Int { willSet {} } } """, expandedSource: """ struct S { var v1: Int @OtherAttribute var v2: Int + var v3: Int { willSet {} } } extension S: RegisterValue { @@ -132,11 +111,84 @@ final class RegisterMacroTests: XCTestCase { line: 4, column: 3, highlight: "@OtherAttribute var v2: Int"), + .init( + message: ErrorDiagnostic.expectedMemberAnnotatedWithMacro(bitFieldMacros).message, + line: 5, + column: 3, + highlight: "var v3: Int { willSet {} }"), ], macros: Self.macros, indentationWidth: Self.indentationWidth) } + func test_members_nonStoredVarDeclsAreOk() { + assertMacroExpansion( + """ + @Register(bitWidth: 0x8) + struct S { + func f() {} + class C {} + var v: Void {} + var v: Void { get {} } + var v: Void { set {} } + var v: Void { _read {} } + var v: Void { _modify {} } + } + """, + expandedSource: """ + struct S { + func f() {} + class C {} + var v: Void {} + var v: Void { get {} } + var v: Void { set {} } + var v: Void { _read {} } + var v: Void { _modify {} } + + private init() { + fatalError() + } + + private var _never: Never + + struct Raw: RegisterValueRaw { + typealias Value = S + typealias Storage = UInt8 + var storage: Storage + init(_ storage: Storage) { + self.storage = storage + } + init(_ value: Value.ReadWrite) { + self.storage = value.storage + } + + } + + typealias Read = ReadWrite + + typealias Write = ReadWrite + + struct ReadWrite: RegisterValueRead, RegisterValueWrite { + typealias Value = S + var storage: UInt8 + init(_ value: ReadWrite) { + self.storage = value.storage + } + init(_ value: Raw) { + self.storage = value.storage + } + + } + } + + extension S: RegisterValue { + } + """, + diagnostics: [], + macros: Self.macros, + indentationWidth: Self.indentationWidth) + } + func test_expansion_noFields() { // FIXME: see expanded source formatting assertMacroExpansion( @@ -190,7 +242,6 @@ final class RegisterMacroTests: XCTestCase { } func test_expansion_noTypedFields() { - // FIXME: see expanded source formatting assertMacroExpansion( """ @Register(bitWidth: 0x8) diff --git a/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/VariableDeclSyntaxTests.swift b/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/VariableDeclSyntaxTests.swift index 9442727c..08716143 100644 --- a/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/VariableDeclSyntaxTests.swift +++ b/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/VariableDeclSyntaxTests.swift @@ -134,4 +134,57 @@ final class VariableDeclSyntaxTests: XCTestCase { } } } + + func test_isComputedProperty() { + struct Vector { + var decl: VariableDeclSyntax + var isComputedProperty: Bool + var file: StaticString + var line: UInt + + init( + decl: DeclSyntax, + isComputedProperty: Bool, + file: StaticString = #file, + line: UInt = #line + ) { + self.decl = decl.as(VariableDeclSyntax.self)! + self.isComputedProperty = isComputedProperty + self.file = file + self.line = line + } + } + + let vectors: [Vector] = [ + .init(decl: "var v: Int", isComputedProperty: false), + .init(decl: "inout v: Int", isComputedProperty: false), + .init(decl: "let v: Int", isComputedProperty: false), + .init(decl: "var v, w: Int", isComputedProperty: false), + .init(decl: "var v: Int, b: Int", isComputedProperty: false), + .init(decl: "var _: Int", isComputedProperty: false), + .init(decl: "var (v, w): Int", isComputedProperty: false), + .init(decl: "var a", isComputedProperty: false), + .init(decl: "var v: _", isComputedProperty: false), + .init(decl: "var v: Int?", isComputedProperty: false), + .init(decl: "var v: [Int]", isComputedProperty: false), + .init(decl: "var v: (Int, Int)", isComputedProperty: false), + .init(decl: "var v: Reg", isComputedProperty: false), + .init(decl: "var v: Swift.Int", isComputedProperty: false), + .init(decl: "var v: Int { willSet {} }", isComputedProperty: false), + .init(decl: "var v: Int { didSet {} }", isComputedProperty: false), + .init(decl: "var v: Void {}", isComputedProperty: true), + .init(decl: "var v: Void { get {} }", isComputedProperty: true), + .init(decl: "var v: Void { set {} }", isComputedProperty: true), + .init(decl: "var v: Void { _read {} }", isComputedProperty: true), + .init(decl: "var v: Void { _modify {} }", isComputedProperty: true), + ] + + for vector in vectors { + XCTAssertEqual( + vector.decl.isComputedProperty, + vector.isComputedProperty, + file: vector.file, + line: vector.line) + } + } }