Merge pull request #76074 from DougGregor/parse-diags-build-config

Author: DougGregor

include/swift/Bridging/ASTGen.h

@@ -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);
 

lib/ASTGen/Sources/ASTGen/ASTGen+CompilerBuildConfiguration.swift

@@ -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)
 

lib/ASTGen/Sources/ASTGen/ASTGen.swift

@@ -98,7 +98,7 @@ struct ASTGenVisitor {
     self.legacyParse = legacyParser
     self.buildConfiguration = CompilerBuildConfiguration(
       ctx: ctx,
-      conditionLoc: BridgedSourceLoc(at: AbsolutePosition(utf8Offset: 0), in: sourceBuffer)
+      sourceBuffer: sourceBuffer
     )
   }
 

lib/ASTGen/Sources/ASTGen/CompilerBuildConfiguration.swift

@@ -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 {
@@ -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
@@ -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
@@ -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)
 

lib/ASTGen/Sources/ASTGen/SourceFile.swift

@@ -12,6 +12,7 @@
 
 import ASTBridging
 import SwiftDiagnostics
+import SwiftIfConfig
 @_spi(ExperimentalLanguageFeatures) import SwiftParser
 import SwiftParserDiagnostics
 import SwiftSyntax
@@ -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,
@@ -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
       }
 

lib/Parse/ParseDecl.cpp

@@ -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 ||
@@ -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() &&

test/ASTGen/if_config.swift

@@ -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)

test/ClangImporter/can_import_underlying_version_tbd_missing_version.swift

@@ -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
 }

test/ClangImporter/can_import_version_ignores_missing_tbd_version.swift

@@ -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
 

test/Parse/ConditionalCompilation/can_import_version_missing_user_module_version.swift

@@ -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
 
 }