From 48e7852483499a81a7e775ff52ebb2b81b0a36ad Mon Sep 17 00:00:00 2001 From: Rauhul Varma Date: Sun, 6 Oct 2024 15:56:58 -0700 Subject: [PATCH] Fix handling of access levels Fixes a bug where access level decl modifiers were getting inserted without a trailing space leading to invalid macro expansions. This bug was caused by a change in overload resolution resulting from the update to swift-syntax 6. Added macro expansion tests as well as type checking tests to cover protect against this sort of bug in the future. --- .../Descriptions/RegisterDescription.swift | 10 +- .../Macros/RegisterBlockMacro.swift | 11 +- Sources/MMIOMacros/Macros/RegisterMacro.swift | 5 +- .../SyntaxStringInterpolation.swift | 11 +- .../RegisterBlockAndOffsetMacroTests.swift | 59 +++++++++ .../Macros/RegisterMacroTests.swift | 118 ++++++++++++++++++ .../SyntaxStringInterpolationTests.swift | 20 +-- Tests/MMIOTests/ExpansionTypeCheckTests.swift | 4 +- 8 files changed, 197 insertions(+), 41 deletions(-) diff --git a/Sources/MMIOMacros/Macros/Descriptions/RegisterDescription.swift b/Sources/MMIOMacros/Macros/Descriptions/RegisterDescription.swift index e642c01d..dd75ff06 100644 --- a/Sources/MMIOMacros/Macros/Descriptions/RegisterDescription.swift +++ b/Sources/MMIOMacros/Macros/Descriptions/RegisterDescription.swift @@ -104,8 +104,8 @@ extension RegisterDescription { \(self.accessLevel)init(_ storage: Storage) { self.storage = storage } - \(initDeclarations) - \(bitFieldDeclarations) + \(nodes: initDeclarations) + \(nodes: bitFieldDeclarations) } """) return declarations @@ -136,7 +136,7 @@ extension RegisterDescription { \(self.accessLevel)init(_ value: Raw) { self.storage = value.storage } - \(bitFieldDeclarations) + \(nodes: bitFieldDeclarations) } """) return declarations @@ -157,7 +157,7 @@ extension RegisterDescription { \(self.accessLevel)typealias Value = \(self.name) var storage: UInt\(raw: self.bitWidth) \(self.accessLevel)init(_ value: Raw) { self.storage = value.storage } - \(bitFieldDeclarations) + \(nodes: bitFieldDeclarations) } """) return declarations @@ -184,7 +184,7 @@ extension RegisterDescription { // FIXME: mask off bits self.storage = value.storage } - \(bitFieldDeclarations) + \(nodes: bitFieldDeclarations) } """) return declarations diff --git a/Sources/MMIOMacros/Macros/RegisterBlockMacro.swift b/Sources/MMIOMacros/Macros/RegisterBlockMacro.swift index 283e9738..adbd07f6 100644 --- a/Sources/MMIOMacros/Macros/RegisterBlockMacro.swift +++ b/Sources/MMIOMacros/Macros/RegisterBlockMacro.swift @@ -63,25 +63,26 @@ extension RegisterBlockMacro: 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?.trimmed + var accessLevel = structDecl.accessLevel?.trimmed + accessLevel?.trailingTrivia = .spaces(1) return [ - "\(acl) let unsafeAddress: UInt", + "\(accessLevel)let unsafeAddress: UInt", """ #if FEATURE_INTERPOSABLE - var interposer: (any MMIOInterposer)? + \(accessLevel)var interposer: (any MMIOInterposer)? #endif """, """ #if FEATURE_INTERPOSABLE @inlinable @inline(__always) - \(acl)init(unsafeAddress: UInt, interposer: (any MMIOInterposer)?) { + \(accessLevel)init(unsafeAddress: UInt, interposer: (any MMIOInterposer)?) { self.unsafeAddress = unsafeAddress self.interposer = interposer } #else @inlinable @inline(__always) - \(acl)init(unsafeAddress: UInt) { + \(accessLevel)init(unsafeAddress: UInt) { self.unsafeAddress = unsafeAddress } #endif diff --git a/Sources/MMIOMacros/Macros/RegisterMacro.swift b/Sources/MMIOMacros/Macros/RegisterMacro.swift index 7663a7b4..ce1459a8 100644 --- a/Sources/MMIOMacros/Macros/RegisterMacro.swift +++ b/Sources/MMIOMacros/Macros/RegisterMacro.swift @@ -44,7 +44,8 @@ extension RegisterMacro: MMIOMemberMacro { ) throws -> [DeclSyntax] { // Can only applied to structs. let structDecl = try declaration.requireAs(StructDeclSyntax.self, context) - let accessLevel = structDecl.accessLevel?.trimmed + var accessLevel = structDecl.accessLevel?.trimmed + accessLevel?.trailingTrivia = .spaces(1) let bitWidth = self.bitWidth.value // Walk all the members of the struct. @@ -100,7 +101,7 @@ extension RegisterMacro: MMIOMemberMacro { let register = RegisterDescription( name: structDecl.name, - accessLevel: structDecl.accessLevel, + accessLevel: accessLevel, bitWidth: self.bitWidth.value, bitFields: bitFields, isSymmetric: isSymmetric) diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/SyntaxStringInterpolation.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/SyntaxStringInterpolation.swift index 0eba81c9..a8a7490d 100644 --- a/Sources/MMIOMacros/SwiftSyntaxExtensions/SyntaxStringInterpolation.swift +++ b/Sources/MMIOMacros/SwiftSyntaxExtensions/SyntaxStringInterpolation.swift @@ -14,16 +14,7 @@ import SwiftSyntaxBuilder extension SyntaxStringInterpolation { mutating func appendInterpolation( - _ node: (some SyntaxProtocol)?, - trailingTrivia: Trivia = .space - ) { - guard let node = node else { return } - self.appendInterpolation(node) - self.appendInterpolation(trailingTrivia) - } - - mutating func appendInterpolation( - _ nodes: [some SyntaxProtocol], + nodes: [some SyntaxProtocol], intermediateTrivia: Trivia = .newline ) { guard let first = nodes.first else { return } diff --git a/Tests/MMIOMacrosTests/Macros/RegisterBlockAndOffsetMacroTests.swift b/Tests/MMIOMacrosTests/Macros/RegisterBlockAndOffsetMacroTests.swift index 945d1a6d..50aa2fbf 100644 --- a/Tests/MMIOMacrosTests/Macros/RegisterBlockAndOffsetMacroTests.swift +++ b/Tests/MMIOMacrosTests/Macros/RegisterBlockAndOffsetMacroTests.swift @@ -147,5 +147,64 @@ final class RegisterBlockAndOffsetMacroTests: XCTestCase { macros: Self.arrayMacros, indentationWidth: Self.indentationWidth) } + + func test_accessLevel_propagation() { + assertMacroExpansion( + """ + @RegisterBlockType + public struct I2C { + @RegisterBlock(offset: 0x0) + var control: Control + @RegisterBlock(offset: 0x8) + var dr: Register + } + """, + expandedSource: """ + public struct I2C { + var control: Control { + @inlinable @inline(__always) get { + #if FEATURE_INTERPOSABLE + return .init(unsafeAddress: self.unsafeAddress + (0x0), interposer: self.interposer) + #else + return .init(unsafeAddress: self.unsafeAddress + (0x0)) + #endif + } + } + var dr: Register { + @inlinable @inline(__always) get { + #if FEATURE_INTERPOSABLE + return .init(unsafeAddress: self.unsafeAddress + (0x8), interposer: self.interposer) + #else + return .init(unsafeAddress: self.unsafeAddress + (0x8)) + #endif + } + } + + public let unsafeAddress: UInt + + #if FEATURE_INTERPOSABLE + public var interposer: (any MMIOInterposer)? + #endif + + #if FEATURE_INTERPOSABLE + @inlinable @inline(__always) + public init(unsafeAddress: UInt, interposer: (any MMIOInterposer)?) { + self.unsafeAddress = unsafeAddress + self.interposer = interposer + } + #else + @inlinable @inline(__always) + public init(unsafeAddress: UInt) { + self.unsafeAddress = unsafeAddress + } + #endif + } + + extension I2C: RegisterProtocol { + } + """, + macros: Self.scalarMacros, + indentationWidth: Self.indentationWidth) + } } #endif diff --git a/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift b/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift index 6f4af73f..9c668b1c 100644 --- a/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift +++ b/Tests/MMIOMacrosTests/Macros/RegisterMacroTests.swift @@ -915,5 +915,123 @@ final class RegisterMacroTests: XCTestCase { macros: Self.macros, indentationWidth: Self.indentationWidth) } + + func test_accessLevel_propagation() { + assertMacroExpansion( + """ + @Register(bitWidth: 0x8) + public struct S { + @ReadOnly(bits: 0..<1, as: Bool.self) + var v1: V1 + @WriteOnly(bits: 1..<2, as: Bool.self) + var v2: V2 + } + """, + expandedSource: """ + public struct S { + @available(*, unavailable) + var v1: V1 { + get { + fatalError() + } + } + @available(*, unavailable) + var v2: V2 { + get { + fatalError() + } + } + + private init() { + fatalError() + } + + private var _never: Never + + public enum V1: ContiguousBitField { + public typealias Storage = UInt8 + public static let bitRange = 0 ..< 1 + } + + public enum V2: ContiguousBitField { + public typealias Storage = UInt8 + public static let bitRange = 1 ..< 2 + } + + public struct Raw: RegisterValueRaw { + public typealias Value = S + public typealias Storage = UInt8 + public var storage: Storage + public init(_ storage: Storage) { + self.storage = storage + } + public init(_ value: Value.Read) { + self.storage = value.storage + } + public init(_ value: Value.Write) { + self.storage = value.storage + } + public var v1: UInt8 { + @inlinable @inline(__always) get { + V1.extract(from: self.storage) + } + @inlinable @inline(__always) set { + V1.insert(newValue, into: &self.storage) + } + } + public var v2: UInt8 { + @inlinable @inline(__always) get { + V2.extract(from: self.storage) + } + @inlinable @inline(__always) set { + V2.insert(newValue, into: &self.storage) + } + } + } + + public struct Read: RegisterValueRead { + public typealias Value = S + var storage: UInt8 + public init(_ value: Raw) { + self.storage = value.storage + } + public var v1: Bool { + @inlinable @inline(__always) get { + preconditionMatchingBitWidth(V1.self, Bool.self) + return Bool(storage: self.raw.v1) + } + } + } + + public struct Write: RegisterValueWrite { + public typealias Value = S + var storage: UInt8 + public init(_ value: Raw) { + self.storage = value.storage + } + public init(_ value: Read) { + // FIXME: mask off bits + self.storage = value.storage + } + public var v2: Bool { + @available(*, deprecated, message: "API misuse; read from write view returns the value to be written, not the value initially read.") + @inlinable @inline(__always) get { + preconditionMatchingBitWidth(V2.self, Bool.self) + return Bool(storage: self.raw.v2) + } + @inlinable @inline(__always) set { + preconditionMatchingBitWidth(V2.self, Bool.self) + self.raw.v2 = newValue.storage(Self.Value.Raw.Storage.self) + } + } + } + } + + extension S: RegisterValue { + } + """, + macros: Self.macros, + indentationWidth: Self.indentationWidth) + } } #endif diff --git a/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/SyntaxStringInterpolationTests.swift b/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/SyntaxStringInterpolationTests.swift index c1771dc9..4c32898a 100644 --- a/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/SyntaxStringInterpolationTests.swift +++ b/Tests/MMIOMacrosTests/SwiftSyntaxExtensions/SyntaxStringInterpolationTests.swift @@ -16,24 +16,10 @@ import XCTest @testable import MMIOMacros final class SyntaxStringInterpolationTests: XCTestCase { - func test_appendInterpolationNodeTrailingTrivia_none() { - let expected: DeclSyntax = "struct S {}" - let acl: DeclModifierSyntax? = nil - let actual: DeclSyntax = "\(acl, trailingTrivia: .spaces(2))struct S {}" - XCTAssertEqual(expected.description, actual.description) - } - - func test_appendInterpolationNodeTrailingTrivia_some() { - let expected: DeclSyntax = "private struct S {}" - let acl: DeclModifierSyntax? = .init(name: .keyword(.private)) - let actual: DeclSyntax = "\(acl, trailingTrivia: .spaces(2))struct S {}" - XCTAssertEqual(expected.description, actual.description) - } - func test_appendInterpolationNodesIntermediateTrivia_none() { let expected: DeclSyntax = "struct S {}" let decls: [DeclSyntax] = [] - let actual: DeclSyntax = "struct S {\(decls, intermediateTrivia: .newlines(2))}" + let actual: DeclSyntax = "struct S {\(nodes: decls, intermediateTrivia: .newlines(2))}" XCTAssertEqual(expected.description, actual.description) } @@ -46,7 +32,7 @@ final class SyntaxStringInterpolationTests: XCTestCase { let decls: [DeclSyntax] = ["var x = 1"] let actual: DeclSyntax = """ struct S { - \(decls, intermediateTrivia: .newlines(2)) + \(nodes: decls, intermediateTrivia: .newlines(2)) } """ XCTAssertEqual(expected.description, actual.description) @@ -63,7 +49,7 @@ final class SyntaxStringInterpolationTests: XCTestCase { let decls: [DeclSyntax] = ["var x = 1", "var y = 2"] let actual: DeclSyntax = """ struct S { - \(decls, intermediateTrivia: .newlines(2)) + \(nodes: decls, intermediateTrivia: .newlines(2)) } """ XCTAssertEqual(expected.description, actual.description) diff --git a/Tests/MMIOTests/ExpansionTypeCheckTests.swift b/Tests/MMIOTests/ExpansionTypeCheckTests.swift index b199b06d..609c8d3c 100644 --- a/Tests/MMIOTests/ExpansionTypeCheckTests.swift +++ b/Tests/MMIOTests/ExpansionTypeCheckTests.swift @@ -13,7 +13,7 @@ import MMIO // Sample register from an STM32F746 @Register(bitWidth: 32) -struct OTG_HPRT { +public struct OTG_HPRT { @ReadWrite(bits: 0..<1) var pcsts: PCSTS @ReadWrite(bits: 1..<2) @@ -81,7 +81,7 @@ struct OtherRangeTypes2 { } @RegisterBlock -struct Block { +public struct Block { @RegisterBlock(offset: 0x4) var otgHprt: Register @RegisterBlock(offset: 0x8, stride: 0x10, count: 100)