Skip to content

Initial support for tracking locations for assigned values in XCConfigs #513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Initial support for tracking locations for assigned values in XCConfigs #513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions Sources/SWBCore/MacroConfigFileLoader.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ final class MacroConfigFileLoader: Sendable {
return MacroConfigFileParser(byteString: data, path: path, delegate: delegate)
}

mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, parser: MacroConfigFileParser) {
mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
// Look up the macro name, creating it as a user-defined macro if it isn’t already known.
let macro = table.namespace.lookupOrDeclareMacro(UserDefinedMacroDeclaration.self, macroName)

Expand All @@ -253,7 +253,8 @@ final class MacroConfigFileLoader: Sendable {
}

// Parse the value in a manner consistent with the macro definition.
table.push(macro, table.namespace.parseForMacro(macro, value: value), conditions: conditionSet)
let location = MacroValueAssignmentLocation(path: path, line: line, startColumn: startColumn, endColumn: endColumn)
table.push(macro, table.namespace.parseForMacro(macro, value: value), conditions: conditionSet, location: location)
}

func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) {
Expand Down Expand Up @@ -301,8 +302,8 @@ fileprivate final class MacroValueAssignmentTableRef {
table.namespace
}

func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil) {
table.push(macro, value, conditions: conditions)
func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil, location: MacroValueAssignmentLocation? = nil) {
table.push(macro, value, conditions: conditions, location: location)
}
}

Expand Down
8 changes: 5 additions & 3 deletions Sources/SWBMacro/MacroConfigFileParser.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ public final class MacroConfigFileParser {
// MARK: Parsing of value assignment starts here.
/// Parses a macro value assignment line of the form MACRONAME [ optional conditions ] ... = VALUE ';'?
private func parseMacroValueAssignment() {
let startOfLine = currIdx - 1
// First skip over any whitespace and comments.
skipWhitespaceAndComments()

Expand Down Expand Up @@ -361,6 +362,7 @@ public final class MacroConfigFileParser {
// Skip over the equals sign.
assert(currChar == /* '=' */ 61)
advance()
let startColumn = currIdx - startOfLine

var chunks : [String] = []
while let chunk = parseNonListAssignmentRHS() {
Expand All @@ -383,7 +385,7 @@ public final class MacroConfigFileParser {
}
// Finally, now that we have the name, conditions, and value, we tell the delegate about it.
let value = chunks.joined(separator: " ")
delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, parser: self)
delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, path: path, line: currLine, startColumn: startColumn, endColumn: currIdx - startOfLine, parser: self)
}

public func parseNonListAssignmentRHS() -> String? {
Expand Down Expand Up @@ -518,7 +520,7 @@ public final class MacroConfigFileParser {
}
func endPreprocessorInclusion() {
}
func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, parser: MacroConfigFileParser) {
func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
self.macroName = macroName
self.conditions = conditions.isEmpty ? nil : conditions
}
Expand Down Expand Up @@ -565,7 +567,7 @@ public protocol MacroConfigFileParserDelegate {
func endPreprocessorInclusion()

/// Invoked once for each macro value assignment. The `macroName` is guaranteed to be non-empty, but `value` may be empty. Any macro conditions are passed as tuples in the `conditions`; parameters are guaranteed to be non-empty strings, but patterns may be empty.
mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, parser: MacroConfigFileParser)
mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser)

/// Invoked if an error, warning, or other diagnostic is detected.
func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser)
Expand Down
70 changes: 64 additions & 6 deletions Sources/SWBMacro/MacroValueAssignmentTable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,21 @@ public struct MacroValueAssignmentTable: Serializable, Sendable {
/// Maps macro declarations to corresponding linked lists of assignments.
public var valueAssignments: [MacroDeclaration: MacroValueAssignment]

private init(namespace: MacroNamespace, valueAssignments: [MacroDeclaration: MacroValueAssignment]) {
public var valueLocations: [String: MacroValueAssignmentLocation]

private init(namespace: MacroNamespace, valueAssignments: [MacroDeclaration: MacroValueAssignment], valueLocations: [String: MacroValueAssignmentLocation]) {
self.namespace = namespace
self.valueAssignments = valueAssignments
self.valueLocations = valueLocations
}

public init(namespace: MacroNamespace) {
self.init(namespace: namespace, valueAssignments: [:])
self.init(namespace: namespace, valueAssignments: [:], valueLocations: [:])
}

/// Convenience initializer to create a `MacroValueAssignmentTable` from another instance (i.e., to create a copy).
public init(copying table: MacroValueAssignmentTable) {
self.init(namespace: table.namespace, valueAssignments: table.valueAssignments)
self.init(namespace: table.namespace, valueAssignments: table.valueAssignments, valueLocations: table.valueLocations)
}

/// Remove all assignments for the given macro.
Expand Down Expand Up @@ -77,18 +80,23 @@ public struct MacroValueAssignmentTable: Serializable, Sendable {


/// Adds a mapping from `macro` to `value`, inserting it ahead of any already existing assignment for the same macro. Unless the value refers to the lower-precedence expression (using `$(inherited)` notation), any existing assignments are shadowed but not removed.
public mutating func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil) {
public mutating func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil, location: MacroValueAssignmentLocation? = nil) {
assert(namespace.lookupMacroDeclaration(macro.name) === macro)
// Validate the type.
assert(macro.type.matchesExpressionType(value))
valueAssignments[macro] = MacroValueAssignment(expression: value, conditions: conditions, next: valueAssignments[macro])

if let location {
valueLocations[macro.name] = location
}
}

/// Adds a mapping from each of the macro-to-value mappings in `otherTable`, inserting them ahead of any already existing assignments in the receiving table. The other table isn’t affected in any way (in particular, no reference is kept from the receiver to the other table).
public mutating func pushContentsOf(_ otherTable: MacroValueAssignmentTable) {
for (macro, firstAssignment) in otherTable.valueAssignments {
valueAssignments[macro] = insertCopiesOfMacroValueAssignmentNodes(firstAssignment, inFrontOf: valueAssignments[macro])
}
valueLocations.merge(otherTable.valueLocations, uniquingKeysWith: { a, b in a })
}

/// Looks up and returns the first (highest-precedence) macro value assignment for `macro`, if there is one.
Expand Down Expand Up @@ -192,6 +200,7 @@ public struct MacroValueAssignmentTable: Serializable, Sendable {
bindAndPushAssignment(firstAssignment)

}
table.valueLocations.merge(self.valueLocations) { a, b in a }
return table
}

Expand Down Expand Up @@ -219,7 +228,7 @@ public struct MacroValueAssignmentTable: Serializable, Sendable {
// MARK: Serialization

public func serialize<T: Serializer>(to serializer: T) {
serializer.beginAggregate(1)
serializer.beginAggregate(2)

// We don't directly serialize MacroDeclarations, but rather serialize their contents "by hand" so when we deserialize we can re-use existing declarations in our namespace.
serializer.beginAggregate(valueAssignments.count)
Expand Down Expand Up @@ -247,6 +256,15 @@ public struct MacroValueAssignmentTable: Serializable, Sendable {
}
serializer.endAggregate() // valueAssignments

serializer.beginAggregate(valueLocations.count)
for (decl, loc) in valueLocations.sorted(by: { $0.0 < $1.0 }) {
serializer.beginAggregate(2)
serializer.serialize(decl)
serializer.serialize(loc)
serializer.endAggregate()
}
serializer.endAggregate()

serializer.endAggregate() // the whole table
}

Expand All @@ -255,9 +273,10 @@ public struct MacroValueAssignmentTable: Serializable, Sendable {
guard let delegate = deserializer.delegate as? (any MacroValueAssignmentTableDeserializerDelegate) else { throw DeserializerError.invalidDelegate("delegate must be a MacroValueAssignmentTableDeserializerDelegate") }
self.namespace = delegate.namespace
self.valueAssignments = [:]
self.valueLocations = [:]

// Deserialize the table.
try deserializer.beginAggregate(1)
try deserializer.beginAggregate(2)

// Iterate over all the key-value pairs.
let count: Int = try deserializer.beginAggregate()
Expand Down Expand Up @@ -304,6 +323,14 @@ public struct MacroValueAssignmentTable: Serializable, Sendable {
// Add it to the dictionary.
self.valueAssignments[decl] = asgn
}

let count2 = try deserializer.beginAggregate()
for _ in 0..<count2 {
try deserializer.beginAggregate(2)
let name: String = try deserializer.deserialize()
let location: MacroValueAssignmentLocation = try deserializer.deserialize()
self.valueLocations[name] = location
}
}
}

Expand Down Expand Up @@ -396,6 +423,37 @@ public final class MacroValueAssignment: Serializable, CustomStringConvertible,
}
}

public struct MacroValueAssignmentLocation: Serializable, Sendable {
public let path: Path
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we're going to end up wanting to intern these strings somewhere to cut down on memory usage in projects with large settings tables

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah, that's a good point. I think settings already track paths of XCConfigs for invalidation purposes, maybe we can re-use that instead of duplicating the paths per macro like I am doing here.

public let line: Int
public let startColumn: Int
public let endColumn: Int

public init(path: Path, line: Int, startColumn: Int, endColumn: Int) {
self.path = path
self.line = line
self.startColumn = startColumn
self.endColumn = endColumn
}

public func serialize<T>(to serializer: T) where T : SWBUtil.Serializer {
serializer.beginAggregate(4)
serializer.serialize(path)
serializer.serialize(line)
serializer.serialize(startColumn)
serializer.serialize(endColumn)
serializer.endAggregate()
}

public init(from deserializer: any SWBUtil.Deserializer) throws {
try deserializer.beginAggregate(4)
self.path = try deserializer.deserialize()
self.line = try deserializer.deserialize()
self.startColumn = try deserializer.deserialize()
self.endColumn = try deserializer.deserialize()
}
}

/// Private function that inserts a copy of the given linked list of MacroValueAssignments (starting at `srcAsgn`) in front of `dstAsgn` (which is optional). The order of the copies is the same as the order of the originals, and the last one will have `dstAsgn` as its `next` property. This function returns the copy that corresponds to `srcAsgn` so the client can add a reference to it wherever it sees fit.
private func insertCopiesOfMacroValueAssignmentNodes(_ srcAsgn: MacroValueAssignment, inFrontOf dstAsgn: MacroValueAssignment?) -> MacroValueAssignment {
// If we aren't inserting in front of anything, we can preserve the input as is.
Expand Down
41 changes: 38 additions & 3 deletions Tests/SWBMacroTests/MacroParsingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ fileprivate let testFileData = [
}
func endPreprocessorInclusion() {
}
func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, parser: MacroConfigFileParser) {
func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
}

func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) {
Expand All @@ -804,19 +804,41 @@ fileprivate let testFileData = [
MacroConfigFileParser(byteString: "// [-Wnullability-completeness-on-arrays] \t\t\t(on) Warns about missing nullability annotations on array parameters.", path: Path(""), delegate: delegate).parse()
#expect(delegate.diagnosticMessages == [String]())
}

@Test
func parserProvidesLocationInformation() throws {
TestMacroConfigFileParser("#include \"Multiline.xcconfig\"",
expectedAssignments: [
(macro: "FEATURE_DEFINES_A", conditions: [], value: "$(A) $(B) $(C)"),
(macro: "FEATURE_DEFINES_B", conditions: [], value: "$(D) $(E) $(F)"),
(macro: "FEATURE_DEFINES_C", conditions: [], value: "$(G) $(H)"),
(macro: "FEATURE_DEFINES_D", conditions: [], value: "$(I)")
],
expectedDiagnostics: [],
expectedLocations: [
(macro: "FEATURE_DEFINES_A", path: .init("Multiline.xcconfig"), line: 2, startColumn: 20, endColumn: 37),
(macro: "FEATURE_DEFINES_B", path: .init("Multiline.xcconfig"), line: 5, startColumn: 20, endColumn: 87),
(macro: "FEATURE_DEFINES_C", path: .init("Multiline.xcconfig"), line: 9, startColumn: 20, endColumn: 61),
(macro: "FEATURE_DEFINES_D", path: .init("Multiline.xcconfig"), line: 11, startColumn: 20, endColumn: 45),
],
expectedIncludeDirectivesCount: 1
)
}
}

// We used typealiased tuples for simplicity and readability.
typealias ConditionInfo = (param: String, pattern: String)
typealias AssignmentInfo = (macro: String, conditions: [ConditionInfo], value: String)
typealias DiagnosticInfo = (level: MacroConfigFileDiagnostic.Level, kind: MacroConfigFileDiagnostic.Kind, line: Int)
typealias LocationInfo = (macro: String, path: Path, line: Int, startColumn: Int, endColumn: Int)

private func TestMacroConfigFileParser(_ string: String, expectedAssignments: [AssignmentInfo], expectedDiagnostics: [DiagnosticInfo], expectedIncludeDirectivesCount: Int, sourceLocation: SourceLocation = #_sourceLocation) {
private func TestMacroConfigFileParser(_ string: String, expectedAssignments: [AssignmentInfo], expectedDiagnostics: [DiagnosticInfo], expectedLocations: [LocationInfo]? = nil, expectedIncludeDirectivesCount: Int, sourceLocation: SourceLocation = #_sourceLocation) {

/// We use a custom delegate to test that we’re getting the expected results, which for the sake of convenience are just kept in (name, conds:[(cond-param, cond-value)], value) tuples, i.e. conditions is an array of two-element tuples.
class ConfigFileParserTestDelegate : MacroConfigFileParserDelegate {
var assignments = Array<AssignmentInfo>()
var diagnostics = Array<DiagnosticInfo>()
var locations = Array<LocationInfo>()

var includeDirectivesCount = 0

Expand All @@ -834,9 +856,10 @@ private func TestMacroConfigFileParser(_ string: String, expectedAssignments: [A
func endPreprocessorInclusion() {
self.includeDirectivesCount += 1
}
func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, parser: MacroConfigFileParser) {
func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
// print("\(parser.lineNumber): \(macroName)\(conditions.map({ "[\($0.param)=\($0.pattern)]" }).joinWithSeparator(""))=\(value)")
assignments.append((macro: macroName, conditions: conditions, value: value))
locations.append((macro: macroName, path: path, line: line, startColumn: startColumn, endColumn: endColumn))
}
func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) {
// print("\(parser.lineNumber): \(diagnostic)")
Expand All @@ -857,6 +880,10 @@ private func TestMacroConfigFileParser(_ string: String, expectedAssignments: [A
// Check the diagnostics that the delegate saw against the expected ones.
#expect(delegate.diagnostics == expectedDiagnostics, "expected parse diagnostics \(expectedDiagnostics), but instead got \(delegate.diagnostics)", sourceLocation: sourceLocation)

if let expectedLocations {
#expect(delegate.locations == expectedLocations, "expected parse locations \(expectedLocations), but instead ogt \(delegate.locations)", sourceLocation: sourceLocation)
}

#expect(delegate.includeDirectivesCount == expectedIncludeDirectivesCount, "expected number of configs parsed to be \(expectedIncludeDirectivesCount), but instead got \(delegate.includeDirectivesCount)", sourceLocation: sourceLocation)
}

Expand Down Expand Up @@ -885,6 +912,14 @@ func ==(lhs: [DiagnosticInfo], rhs: [DiagnosticInfo]) -> Bool {
return lhs.count == rhs.count && zip(lhs, rhs).filter({ return !($0.0 == $0.1) }).isEmpty
}

func ==(lhs: LocationInfo, rhs: LocationInfo) -> Bool {
return (lhs.macro == rhs.macro) && (lhs.path == rhs.path) && (lhs.line == rhs.line) && (lhs.startColumn == rhs.startColumn) && (lhs.endColumn == rhs.endColumn)
}

func ==(lhs: [LocationInfo], rhs: [LocationInfo]) -> Bool {
return lhs.count == rhs.count && zip(lhs, rhs).filter({ return !($0.0 == $0.1) }).isEmpty
}


/// Private helper function that parses a string representation as either a string or a string list (depending on the parameter), and checks the resulting parser delegate method call sequence and diagnostics (if applicable) against what’s expected. This is a private function that’s called by the two internal test functions TestMacroStringParsing() and TestMacroStringListParsing(). The original file name and line number are passed in so that Xcode diagnostics will refer to the call site. Each diagnostic is provided by the unit test as a tuple containing the level, kind, and associated range (expressed as start and end “distances”, in the manner of Int.Distance, into the original string).
private func TestMacroParsing(_ string: String, asList: Bool, expectedCallLogEntries: [ParseDelegateCallLogEntry], expectedDiagnosticInfos: [(level: MacroExpressionDiagnostic.Level, kind: MacroExpressionDiagnostic.Kind, start: Int, end: Int)], sourceLocation: SourceLocation = #_sourceLocation) {
Expand Down
Loading