Skip to content

Suppress certain warnings in inactive #if regions without relying on IfConfigDecl #76327

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.

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ extension ASTGenVisitor {
/// produced due to the evaluation.
func activeClause(in node: IfConfigDeclSyntax) -> IfConfigClauseSyntax? {
// Determine the active clause.
let (activeClause, diagnostics) = node.activeClause(in: buildConfiguration)
diagnoseAll(diagnostics)

return activeClause
return configuredRegions.activeClause(for: node)
}

/// Visit a collection of elements that may contain #if clauses within it
Expand Down
33 changes: 19 additions & 14 deletions lib/ASTGen/Sources/ASTGen/ASTGen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import ASTBridging
import BasicBridging
import ParseBridging
import SwiftIfConfig
// Needed to use BumpPtrAllocator
@_spi(BumpPtrAllocator) @_spi(RawSyntax) import SwiftSyntax

Expand Down Expand Up @@ -76,7 +77,7 @@ struct ASTGenVisitor {

let ctx: BridgedASTContext

let buildConfiguration: CompilerBuildConfiguration
let configuredRegions: ConfiguredRegions

fileprivate let allocator: SwiftSyntax.BumpPtrAllocator = .init(initialSlabSize: 256)

Expand All @@ -89,17 +90,15 @@ struct ASTGenVisitor {
sourceBuffer: UnsafeBufferPointer<UInt8>,
declContext: BridgedDeclContext,
astContext: BridgedASTContext,
configuredRegions: ConfiguredRegions,
legacyParser: BridgedLegacyParser
) {
self.diagnosticEngine = diagnosticEngine
self.base = sourceBuffer
self.declContext = declContext
self.ctx = astContext
self.configuredRegions = configuredRegions
self.legacyParse = legacyParser
self.buildConfiguration = CompilerBuildConfiguration(
ctx: ctx,
sourceBuffer: sourceBuffer
)
}

func generate(sourceFile node: SourceFileSyntax) -> [BridgedDecl] {
Expand Down Expand Up @@ -423,31 +422,36 @@ extension TokenSyntax {
@_cdecl("swift_ASTGen_buildTopLevelASTNodes")
public func buildTopLevelASTNodes(
diagEngine: BridgedDiagnosticEngine,
sourceFilePtr: UnsafeRawPointer,
sourceFilePtr: UnsafeMutableRawPointer,
dc: BridgedDeclContext,
ctx: BridgedASTContext,
legacyParser: BridgedLegacyParser,
outputContext: UnsafeMutableRawPointer,
callback: @convention(c) (UnsafeMutableRawPointer, UnsafeMutableRawPointer) -> Void
) {
let sourceFile = sourceFilePtr.assumingMemoryBound(to: ExportedSourceFile.self)
ASTGenVisitor(
let visitor = ASTGenVisitor(
diagnosticEngine: diagEngine,
sourceBuffer: sourceFile.pointee.buffer,
declContext: dc,
astContext: ctx,
configuredRegions: sourceFile.pointee.configuredRegions(astContext: ctx),
legacyParser: legacyParser
)
.generate(sourceFile: sourceFile.pointee.syntax)
.forEach { callback($0.raw, outputContext) }

visitor.generate(sourceFile: sourceFile.pointee.syntax)
.forEach { callback($0.raw, outputContext) }

// Diagnose any errors from evaluating #ifs.
visitor.diagnoseAll(visitor.configuredRegions.diagnostics)
}

/// Generate an AST node at the given source location. Returns the generated
/// ASTNode and mutate the pointee of `endLocPtr` to the end of the node.
private func _build<Node: SyntaxProtocol, Result>(
generator: (ASTGenVisitor) -> (Node) -> Result,
diagEngine: BridgedDiagnosticEngine,
sourceFilePtr: UnsafeRawPointer,
sourceFilePtr: UnsafeMutableRawPointer,
sourceLoc: BridgedSourceLoc,
declContext: BridgedDeclContext,
astContext: BridgedASTContext,
Expand Down Expand Up @@ -480,6 +484,7 @@ private func _build<Node: SyntaxProtocol, Result>(
sourceBuffer: sourceFile.pointee.buffer,
declContext: declContext,
astContext: astContext,
configuredRegions: sourceFile.pointee.configuredRegions(astContext: astContext),
legacyParser: legacyParser
)
)(node)
Expand All @@ -489,7 +494,7 @@ private func _build<Node: SyntaxProtocol, Result>(
@usableFromInline
func buildTypeRepr(
diagEngine: BridgedDiagnosticEngine,
sourceFilePtr: UnsafeRawPointer,
sourceFilePtr: UnsafeMutableRawPointer,
sourceLoc: BridgedSourceLoc,
declContext: BridgedDeclContext,
astContext: BridgedASTContext,
Expand All @@ -512,7 +517,7 @@ func buildTypeRepr(
@usableFromInline
func buildDecl(
diagEngine: BridgedDiagnosticEngine,
sourceFilePtr: UnsafeRawPointer,
sourceFilePtr: UnsafeMutableRawPointer,
sourceLoc: BridgedSourceLoc,
declContext: BridgedDeclContext,
astContext: BridgedASTContext,
Expand All @@ -535,7 +540,7 @@ func buildDecl(
@usableFromInline
func buildExpr(
diagEngine: BridgedDiagnosticEngine,
sourceFilePtr: UnsafeRawPointer,
sourceFilePtr: UnsafeMutableRawPointer,
sourceLoc: BridgedSourceLoc,
declContext: BridgedDeclContext,
astContext: BridgedASTContext,
Expand All @@ -558,7 +563,7 @@ func buildExpr(
@usableFromInline
func buildStmt(
diagEngine: BridgedDiagnosticEngine,
sourceFilePtr: UnsafeRawPointer,
sourceFilePtr: UnsafeMutableRawPointer,
sourceLoc: BridgedSourceLoc,
declContext: BridgedDeclContext,
astContext: BridgedASTContext,
Expand Down
152 changes: 146 additions & 6 deletions lib/ASTGen/Sources/ASTGen/CompilerBuildConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -179,20 +179,33 @@ enum IfConfigError: Error, CustomStringConvertible {
}
}

extension ExportedSourceFile {
/// Return the configured regions for this source file.
mutating func configuredRegions(astContext: BridgedASTContext) -> ConfiguredRegions {
if let _configuredRegions {
return _configuredRegions
}

let configuration = CompilerBuildConfiguration(
ctx: astContext,
sourceBuffer: buffer
)

let regions = syntax.configuredRegions(in: configuration)
_configuredRegions = regions
return regions
}
}

/// Extract the #if clause range information for the given source file.
@_cdecl("swift_ASTGen_configuredRegions")
public func configuredRegions(
astContext: BridgedASTContext,
sourceFilePtr: UnsafeRawPointer,
sourceFilePtr: UnsafeMutableRawPointer,
cRegionsOut: UnsafeMutablePointer<UnsafeMutablePointer<BridgedIfConfigClauseRangeInfo>?>
) -> Int {
let sourceFilePtr = sourceFilePtr.bindMemory(to: ExportedSourceFile.self, capacity: 1)
let configuration = CompilerBuildConfiguration(
ctx: astContext,
sourceBuffer: sourceFilePtr.pointee.buffer
)
let regions = sourceFilePtr.pointee.syntax.configuredRegions(in: configuration)
let regions = sourceFilePtr.pointee.configuredRegions(astContext: astContext)

var cRegions: [BridgedIfConfigClauseRangeInfo] = []

Expand Down Expand Up @@ -321,6 +334,133 @@ public func freeConfiguredRegions(
UnsafeMutableBufferPointer(start: regions, count: numRegions).deallocate()
}

/// Cache used when checking for inactive code that might contain a reference
/// to specific names.
private struct InactiveCodeContainsReferenceCache {
let syntax: SourceFileSyntax
let configuredRegions: ConfiguredRegions
}

/// Search in inactive/unparsed code to look for evidence that something that
/// looks unused is actually used in some configurations.
private enum InactiveCodeChecker {
case name(String)
Copy link
Member

Choose a reason for hiding this comment

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

Should this take an Identifier already instead of a String?

Suggested change
case name(String)
case name(Identifier)

Copy link
Member Author

Choose a reason for hiding this comment

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

Identifier needs either SyntaxText in an arena, or a StaticString. I don't really have either here because I'm coming from a bridged StringRef.

case tryOrThrow

/// Search for the kind of entity in the given syntax node.
func search(syntax: SourceFileSyntax, configuredRegions: ConfiguredRegions) -> Bool {
// If there are no regions, everything is active. This is the common case.
if configuredRegions.isEmpty {
return false
}

for token in syntax.tokens(viewMode: .sourceAccurate) {
Copy link
Member

Choose a reason for hiding this comment

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

Using a SyntaxVisitor instead of tokens(viewMode:) is currently faster because it has performance optimizations to reuse memory allocations, in case this is relevant here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's harder to stop the visitor from running when we find something, though, whereas I can easily "break" out of loop that walks over tokens.

// Match what we're looking for, and continue iterating if it doesn't
// match.
switch self {
case .name(let name):
guard let identifier = token.identifier, identifier.name == name else {
continue
}

break

case .tryOrThrow:
guard let keywordKind = token.keywordKind,
keywordKind == .try || keywordKind == .throw else {
continue
}

break
}

// We matched what we were looking for, now check whether it is in an
// inactive or unparsed region.
if configuredRegions.isActive(token) != .active {
// Found it in a non-active region.
return true
}
}

return false
}
}

/// Determine whether the inactive code within the given search range
/// contains a token referring to the given name.
@_cdecl("swift_ASTGen_inactiveCodeContainsReference")
public func inactiveCodeContainsReference(
astContext: BridgedASTContext,
sourceFileBuffer: BridgedStringRef,
searchRange: BridgedStringRef,
bridgedName: BridgedStringRef,
untypedCachePtr: UnsafeMutablePointer<UnsafeMutableRawPointer?>
) -> Bool {
let syntax: SourceFileSyntax
let configuredRegions: ConfiguredRegions
if let untypedCachePtr = untypedCachePtr.pointee {
// Use the cache.
let cache = untypedCachePtr.assumingMemoryBound(to: InactiveCodeContainsReferenceCache.self)
syntax = cache.pointee.syntax
configuredRegions = cache.pointee.configuredRegions
} else {
// Parse the region.

// FIXME: Use 'ExportedSourceFile' when C++ parser is replaced.
let searchRangeBuffer = UnsafeBufferPointer<UInt8>(start: searchRange.data, count: searchRange.count)
syntax = Parser.parse(source: searchRangeBuffer)

// Compute the configured regions within the search range.
let configuration = CompilerBuildConfiguration(
ctx: astContext,
sourceBuffer: searchRangeBuffer
)
configuredRegions = syntax.configuredRegions(in: configuration)

let cache = UnsafeMutablePointer<InactiveCodeContainsReferenceCache>.allocate(capacity: 1)
cache.initialize(to: .init(syntax: syntax, configuredRegions: configuredRegions))

untypedCachePtr.pointee = .init(cache)
}

return InactiveCodeChecker.name(String(bridged: bridgedName))
.search(syntax: syntax, configuredRegions: configuredRegions)
}

@_cdecl("swift_ASTGen_inactiveCodeContainsTryOrThrow")
public func inactiveCodeContainsTryOrThrow(
astContext: BridgedASTContext,
sourceFileBuffer: BridgedStringRef,
searchRange: BridgedStringRef
) -> Bool {
// Parse the region.

// FIXME: Use 'ExportedSourceFile' when C++ parser is replaced.
let searchRangeBuffer = UnsafeBufferPointer<UInt8>(start: searchRange.data, count: searchRange.count)
let syntax = Parser.parse(source: searchRangeBuffer)

// Compute the configured regions within the search range.
let configuration = CompilerBuildConfiguration(
ctx: astContext,
sourceBuffer: searchRangeBuffer
)
let configuredRegions = syntax.configuredRegions(in: configuration)

return InactiveCodeChecker.tryOrThrow
.search(syntax: syntax, configuredRegions: configuredRegions)
}

@_cdecl("swift_ASTGen_freeInactiveCodeContainsReferenceCache")
public func freeInactiveCodeContainsReferenceCache(pointer: UnsafeMutableRawPointer?) {
guard let pointer else {
return
}

let cache = pointer.assumingMemoryBound(to: InactiveCodeContainsReferenceCache.self)
cache.deinitialize(count: 1)
cache.deallocate()
}

/// Evaluate the #if condition at ifClauseLocationPtr.
@_cdecl("swift_ASTGen_evaluatePoundIfCondition")
public func evaluatePoundIfCondition(
Expand Down
12 changes: 6 additions & 6 deletions lib/ASTGen/Sources/ASTGen/SourceFile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ public struct ExportedSourceFile {
/// Cached so we don't need to re-build the line table every time we need to convert a position.
let sourceLocationConverter: SourceLocationConverter

/// Configured regions for this source file.
///
/// This is a cached value; access via configuredRegions(astContext:).
var _configuredRegions: ConfiguredRegions? = nil

public func position(of location: BridgedSourceLoc) -> AbsolutePosition? {
let sourceFileBaseAddress = UnsafeRawPointer(buffer.baseAddress!)
guard let opaqueValue = location.getOpaquePointerValue() else {
Expand Down Expand Up @@ -162,12 +167,7 @@ public func emitParserDiagnostics(
let diags = ParseDiagnosticsGenerator.diagnostics(for: sourceFileSyntax)

let diagnosticEngine = BridgedDiagnosticEngine(raw: diagEnginePtr)
let buildConfiguration = CompilerBuildConfiguration(
ctx: ctx,
sourceBuffer: sourceFile.pointee.buffer
)

let configuredRegions = sourceFileSyntax.configuredRegions(in: buildConfiguration)
let configuredRegions = sourceFile.pointee.configuredRegions(astContext: ctx)
for diag in diags {
// If the diagnostic is in an unparsed #if region, don't emit it.
if configuredRegions.isActive(diag.node) == .unparsed {
Expand Down
Loading