Skip to content

Use SwiftIfConfig to determine where to emit new parser diagnostics #76074

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
merged 3 commits into from
Aug 24, 2024
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
1 change: 1 addition & 0 deletions include/swift/Bridging/ASTGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ int swift_ASTGen_roundTripCheck(void *_Nonnull sourceFile);
/// Emit parser diagnostics for given source file.. Returns non-zero if any
/// diagnostics were emitted.
int swift_ASTGen_emitParserDiagnostics(
BridgedASTContext astContext,
void *_Nonnull diagEngine, void *_Nonnull sourceFile, int emitOnlyErrors,
int downgradePlaceholderErrorsToWarnings);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ extension ASTGenVisitor {
/// produced due to the evaluation.
func activeClause(in node: IfConfigDeclSyntax) -> IfConfigClauseSyntax? {
// Determine the active clause.
var buildConfiguration = self.buildConfiguration
buildConfiguration.conditionLoc = generateSourceLoc(node)
let (activeClause, diagnostics) = node.activeClause(in: buildConfiguration)
diagnoseAll(diagnostics)

Expand Down
2 changes: 1 addition & 1 deletion lib/ASTGen/Sources/ASTGen/ASTGen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ struct ASTGenVisitor {
self.legacyParse = legacyParser
self.buildConfiguration = CompilerBuildConfiguration(
ctx: ctx,
conditionLoc: BridgedSourceLoc(at: AbsolutePosition(utf8Offset: 0), in: sourceBuffer)
sourceBuffer: sourceBuffer
)
}

Expand Down
20 changes: 13 additions & 7 deletions lib/ASTGen/Sources/ASTGen/CompilerBuildConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import SwiftSyntax
/// queries.
struct CompilerBuildConfiguration: BuildConfiguration {
let ctx: BridgedASTContext
var conditionLoc: BridgedSourceLoc
let sourceBuffer: UnsafeBufferPointer<UInt8>

init(ctx: BridgedASTContext, conditionLoc: BridgedSourceLoc) {
init(ctx: BridgedASTContext, sourceBuffer: UnsafeBufferPointer<UInt8>) {
self.ctx = ctx
self.conditionLoc = conditionLoc
self.sourceBuffer = sourceBuffer
}

func isCustomConditionSet(name: String) throws -> Bool {
Expand All @@ -48,8 +48,11 @@ struct CompilerBuildConfiguration: BuildConfiguration {
}
}

func canImport(importPath: [String], version: CanImportVersion) throws -> Bool {
var importPathStr = importPath.joined(separator: ".")
func canImport(
importPath: [(TokenSyntax, String)],
version: CanImportVersion
) throws -> Bool {
var importPathStr = importPath.map { $0.1 }.joined(separator: ".")

var versionComponents: [Int]
let cVersionKind: BridgedCanImportVersion
Expand All @@ -71,7 +74,10 @@ struct CompilerBuildConfiguration: BuildConfiguration {
versionComponents.withUnsafeBufferPointer { versionComponentsBuf in
ctx.canImport(
importPath: bridgedImportPathStr,
location: conditionLoc,
location: BridgedSourceLoc(
at: importPath.first!.0.position,
in: sourceBuffer
),
versionKind: cVersionKind,
versionComponents: versionComponentsBuf.baseAddress,
numVersionComponents: versionComponentsBuf.count
Expand Down Expand Up @@ -165,7 +171,7 @@ public func configuredRegions(
let sourceFilePtr = sourceFilePtr.bindMemory(to: ExportedSourceFile.self, capacity: 1)
let configuration = CompilerBuildConfiguration(
ctx: astContext,
conditionLoc: sourceFilePtr.pointee.sourceLoc(at: AbsolutePosition(utf8Offset: 0))
sourceBuffer: sourceFilePtr.pointee.buffer
)
let regions = sourceFilePtr.pointee.syntax.configuredRegions(in: configuration)

Expand Down
30 changes: 12 additions & 18 deletions lib/ASTGen/Sources/ASTGen/SourceFile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import ASTBridging
import SwiftDiagnostics
import SwiftIfConfig
@_spi(ExperimentalLanguageFeatures) import SwiftParser
import SwiftParserDiagnostics
import SwiftSyntax
Expand Down Expand Up @@ -142,20 +143,10 @@ public func roundTripCheck(
}
}

extension Syntax {
/// Whether this syntax node is or is enclosed within a #if.
fileprivate var isInIfConfig: Bool {
if self.is(IfConfigDeclSyntax.self) {
return true
}

return parent?.isInIfConfig ?? false
}
}

/// Emit diagnostics within the given source file.
@_cdecl("swift_ASTGen_emitParserDiagnostics")
public func emitParserDiagnostics(
ctx: BridgedASTContext,
diagEnginePtr: UnsafeMutableRawPointer,
sourceFilePtr: UnsafeMutablePointer<UInt8>,
emitOnlyErrors: CInt,
Expand All @@ -167,16 +158,19 @@ public func emitParserDiagnostics(
) { sourceFile in
var anyDiags = false

let diags = ParseDiagnosticsGenerator.diagnostics(
for: sourceFile.pointee.syntax
)
let sourceFileSyntax = sourceFile.pointee.syntax
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)
for diag in diags {
// Skip over diagnostics within #if, because we don't know whether
// we are in an active region or not.
// FIXME: This heuristic could be improved.
if diag.node.isInIfConfig {
// If the diagnostic is in an unparsed #if region, don't emit it.
if configuredRegions.isActive(diag.node) == .unparsed {
continue
}

Expand Down
4 changes: 2 additions & 2 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
if (parsingOpts.contains(ParsingFlags::ValidateNewParserDiagnostics) &&
!Context.Diags.hadAnyError()) {
auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics(
&Context.Diags, exportedSourceFile,
Context, &Context.Diags, exportedSourceFile,
/*emitOnlyErrors=*/true,
/*downgradePlaceholderErrorsToWarnings=*/
Context.LangOpts.Playground ||
Expand Down Expand Up @@ -346,7 +346,7 @@ void Parser::parseSourceFileViaASTGen(
// If we're supposed to emit diagnostics from the parser, do so now.
if (!suppressDiagnostics) {
auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics(
&Context.Diags, exportedSourceFile, /*emitOnlyErrors=*/false,
Context, &Context.Diags, exportedSourceFile, /*emitOnlyErrors=*/false,
/*downgradePlaceholderErrorsToWarnings=*/langOpts.Playground ||
langOpts.WarnOnEditorPlaceholder);
if (hadSyntaxError && Context.Diags.hadAnyError() &&
Expand Down
4 changes: 2 additions & 2 deletions test/ASTGen/if_config.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// REQUIRES: asserts

#if NOT_SET
func f { } // FIXME: Error once the parser diagnostics generator knows to
// evaluate the active clause.
func f { } // expected-error{{expected parameter clause in function signature}}
// expected-note@-1{{insert parameter clause}}{{7-8=}}{{8-8=(}}{{8-8=) }}
#endif

#if compiler(>=10.0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import Simple

func canImportUnderlyingVersion() {
#if canImport(Simple, _underlyingVersion: 3.3) // expected-warning {{cannot find user version number for Clang module 'Simple'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Simple'; version number ignored}}
let a = 1 // expected-warning {{initialization of immutable value 'a' was never used; consider replacing with assignment to '_' or removing it}}
#endif
}

func canImportVersion() {
#if canImport(Simple, _version: 3.3) // expected-warning {{cannot find user version number for Clang module 'Simple'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Simple'; version number ignored}}
let a = 1 // expected-warning {{initialization of immutable value 'a' was never used; consider replacing with assignment to '_' or removing it}}
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@ import Simple

func canImportUnderlyingVersion() {
#if canImport(Simple, _underlyingVersion: 2) // expected-warning {{cannot find user version number for Clang module 'Simple'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Simple'; version number ignored}}
let a = 1 // expected-warning {{initialization of immutable value 'a' was never used; consider replacing with assignment to '_' or removing it}}
#endif

#if canImport(Simple, _underlyingVersion: 3) // expected-warning {{cannot find user version number for Clang module 'Simple'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Simple'; version number ignored}}
let b = 1 // expected-warning {{initialization of immutable value 'b' was never used; consider replacing with assignment to '_' or removing it}}
#endif

#if canImport(Simple, _underlyingVersion: 4) // expected-warning {{cannot find user version number for Clang module 'Simple'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Simple'; version number ignored}}
let c = 1 // expected-warning {{initialization of immutable value 'c' was never used; consider replacing with assignment to '_' or removing it}}
#endif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,17 @@ import NoUserModuleVersion
func testCanImportNoUserModuleVersion() {

#if canImport(NoUserModuleVersion, _version: 113.331) // expected-warning {{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
// NOTE: Duplicate warning because the canImport request happens twice when parser
// validation is enabled.
// TODO(ParserValidation): expected-warning@-3 *{{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
let a = 1 // expected-warning {{initialization of immutable value 'a' was never used; consider replacing with assignment to '_' or removing it}}
#endif

#if canImport(NoUserModuleVersion, _version: 2) // expected-warning {{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
let b = 1 // expected-warning {{initialization of immutable value 'b' was never used; consider replacing with assignment to '_' or removing it}}
// NOTE: Duplicate warning because the canImport request happens twice when parser
// validation is enabled.
// TODO(ParserValidation): expected-warning@-4 *{{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
#endif

}