Merge pull request #73237 from rintaro/macros-checkdefinition-nosourcefile

[Macros] Don't use 'ExportedSourceFile' for macro definition checking

Author: rintaro

include/swift/Bridging/ASTGen.h

@@ -73,8 +73,8 @@ bool swift_ASTGen_checkDefaultArgumentMacroExpression(
     const void *_Nonnull macroSourceLocation);
 
 ptrdiff_t swift_ASTGen_checkMacroDefinition(
-    void *_Nonnull diagEngine, void *_Nonnull sourceFile,
-    const void *_Nonnull macroSourceLocation,
+    void *_Nonnull diagEngine, BridgedStringRef sourceFileBuffer,
+    BridgedStringRef macroDeclText,
     BridgedStringRef *_Nonnull expansionSourceOutPtr,
     ptrdiff_t *_Nullable *_Nonnull replacementsPtr,
     ptrdiff_t *_Nonnull numReplacements,

lib/ASTGen/Sources/ASTGen/DiagnosticsBridge.swift

@@ -21,14 +21,15 @@ fileprivate func emitDiagnosticParts(
   message: String,
   severity: DiagnosticSeverity,
   position: AbsolutePosition,
+  offset: Int,
   highlights: [Syntax] = [],
   fixItChanges: [FixIt.Change] = []
 ) {
   // Map severity
   let bridgedSeverity = severity.bridged
 
   func bridgedSourceLoc(at position: AbsolutePosition) -> BridgedSourceLoc {
-    return BridgedSourceLoc(at: position, in: sourceFileBuffer)
+    return BridgedSourceLoc(at: position.advanced(by: offset), in: sourceFileBuffer)
   }
 
   // Emit the diagnostic
@@ -96,6 +97,7 @@ fileprivate func emitDiagnosticParts(
 func emitDiagnostic(
   diagnosticEngine: BridgedDiagnosticEngine,
   sourceFileBuffer: UnsafeBufferPointer<UInt8>,
+  sourceFileBufferOffset: Int = 0,
   diagnostic: Diagnostic,
   diagnosticSeverity: DiagnosticSeverity,
   messageSuffix: String? = nil
@@ -107,6 +109,7 @@ func emitDiagnostic(
     message: diagnostic.diagMessage.message + (messageSuffix ?? ""),
     severity: diagnosticSeverity,
     position: diagnostic.position,
+    offset: sourceFileBufferOffset,
     highlights: diagnostic.highlights
   )
 
@@ -118,6 +121,7 @@ func emitDiagnostic(
       message: fixIt.message.message,
       severity: .note,
       position: diagnostic.position,
+      offset: sourceFileBufferOffset,
       fixItChanges: fixIt.changes
     )
   }
@@ -129,7 +133,8 @@ func emitDiagnostic(
       sourceFileBuffer: sourceFileBuffer,
       message: note.message,
       severity: .note,
-      position: note.position
+      position: note.position,
+      offset: sourceFileBufferOffset
     )
   }
 }

lib/ASTGen/Sources/ASTGen/Macros.swift

@@ -15,6 +15,7 @@ import BasicBridging
 @_spi(PluginMessage) @_spi(ExperimentalLanguageFeature) import SwiftCompilerPluginMessageHandling
 import SwiftDiagnostics
 import SwiftOperators
+import SwiftParser
 import SwiftSyntax
 import SwiftSyntaxBuilder
 @_spi(ExperimentalLanguageFeature) @_spi(Compiler) import SwiftSyntaxMacroExpansion
@@ -243,8 +244,8 @@ func checkDefaultArgumentMacroExpression(
 @_cdecl("swift_ASTGen_checkMacroDefinition")
 func checkMacroDefinition(
   diagEnginePtr: UnsafeMutableRawPointer,
-  sourceFilePtr: UnsafeRawPointer,
-  macroLocationPtr: UnsafePointer<UInt8>,
+  sourceFileBuffer: BridgedStringRef,
+  macroDeclText: BridgedStringRef,
   externalMacroOutPtr: UnsafeMutablePointer<BridgedStringRef>,
   replacementsPtr: UnsafeMutablePointer<UnsafeMutablePointer<Int>?>,
   numReplacementsPtr: UnsafeMutablePointer<Int>,
@@ -255,20 +256,25 @@ func checkMacroDefinition(
   assert(externalMacroOutPtr.pointee.isEmptyInitialized)
   assert(replacementsPtr.pointee == nil && numReplacementsPtr.pointee == 0)
 
-  let sourceFilePtr = sourceFilePtr.bindMemory(to: ExportedSourceFile.self, capacity: 1)
-
-  // Find the macro declaration.
-  guard
-    let macroDecl = findSyntaxNodeInSourceFile(
-      sourceFilePtr: sourceFilePtr,
-      sourceLocationPtr: macroLocationPtr,
-      type: MacroDeclSyntax.self
-    )
-  else {
+  // Parse 'macro' decl.
+  // FIXME: Use 'ExportedSourceFile' when C++ parser is replaced.
+  let textBuffer = UnsafeBufferPointer<UInt8>(start: macroDeclText.data, count: macroDeclText.count)
+  var parser = Parser(textBuffer)
+  guard let macroDecl = DeclSyntax.parse(from: &parser).as(MacroDeclSyntax.self) else {
     // FIXME: Produce an error
     return -1
   }
 
+  func diagnose(diagnostic: Diagnostic) {
+    emitDiagnostic(
+      diagnosticEngine: BridgedDiagnosticEngine(raw: diagEnginePtr),
+      sourceFileBuffer: UnsafeBufferPointer(start: sourceFileBuffer.data, count: sourceFileBuffer.count),
+      sourceFileBufferOffset: macroDeclText.data! - sourceFileBuffer.data!,
+      diagnostic: diagnostic,
+      diagnosticSeverity: diagnostic.diagMessage.severity
+    )
+  }
+
   // Check the definition
   do {
     let definition = try macroDecl.checkDefinition()
@@ -299,9 +305,7 @@ func checkMacroDefinition(
 
         default:
           // Warn about the unknown builtin.
-          let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr)
-          srcMgr.insert(sourceFilePtr)
-          srcMgr.diagnose(
+          diagnose(
             diagnostic: .init(
               node: node,
               message: ASTGenMacroDiagnostic.unknownBuiltin(type)
@@ -321,9 +325,7 @@ func checkMacroDefinition(
         "#externalMacro(module: \(literal: module), type: \(literal: type))"
 
       // Warn about the use of old-style external macro syntax here.
-      let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr)
-      srcMgr.insert(sourceFilePtr)
-      srcMgr.diagnose(
+      diagnose(
         diagnostic: .init(
           node: node,
           message: ASTGenMacroDiagnostic.oldStyleExternalMacro,
@@ -350,9 +352,7 @@ func checkMacroDefinition(
         firstArgLabel == "module",
         let module = identifierFromStringLiteral(firstArg.expression)
       else {
-        let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr)
-        srcMgr.insert(sourceFilePtr)
-        srcMgr.diagnose(
+        diagnose(
           diagnostic: .init(
             node: Syntax(expansionSyntax),
             message: ASTGenMacroDiagnostic.notStringLiteralArgument("module")
@@ -367,9 +367,7 @@ func checkMacroDefinition(
         secondArgLabel == "type",
         let type = identifierFromStringLiteral(secondArg.expression)
       else {
-        let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr)
-        srcMgr.insert(sourceFilePtr)
-        srcMgr.diagnose(
+        diagnose(
           diagnostic: .init(
             node: Syntax(expansionSyntax),
             message: ASTGenMacroDiagnostic.notStringLiteralArgument("type")
@@ -431,16 +429,12 @@ func checkMacroDefinition(
 #endif
     }
   } catch let errDiags as DiagnosticsError {
-    let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr)
-    srcMgr.insert(sourceFilePtr)
     for diag in errDiags.diagnostics {
-      srcMgr.diagnose(diagnostic: diag)
+      diagnose(diagnostic: diag)
     }
     return -1
   } catch let error {
-    let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr)
-    srcMgr.insert(sourceFilePtr)
-    srcMgr.diagnose(
+    diagnose(
       diagnostic: .init(
         node: Syntax(macroDecl),
         message: ASTGenMacroDiagnostic.thrownError(error)

lib/Sema/TypeCheckMacros.cpp

@@ -33,6 +33,7 @@
 #include "swift/AST/PrettyStackTrace.h"
 #include "swift/AST/SourceFile.h"
 #include "swift/AST/TypeCheckRequests.h"
+#include "swift/Basic/BasicBridging.h"
 #include "swift/Basic/Defer.h"
 #include "swift/Basic/Lazy.h"
 #include "swift/Basic/SourceManager.h"
@@ -150,11 +151,23 @@ MacroDefinition MacroDefinitionRequest::evaluate(
   ptrdiff_t numReplacements = 0;
   ptrdiff_t *genericReplacements = nullptr;
   ptrdiff_t numGenericReplacements = 0;
+
+  // Parse 'macro' decl in swift_ASTGen_checkMacroDefinition.
+  // NOTE: We don't use source->getExportedSourceFile() because it parses the
+  // entire source buffer, which we don't want. Usually 'macro' decl is not in
+  // the same file as the expansion, so we only want to parse only the decl.
+  // FIXME: When we migrate to SwiftParser, use the parsed syntax tree.
+  auto &SM = ctx.SourceMgr;
+  StringRef sourceFileText =
+      SM.getEntireTextForBuffer(*sourceFile->getBufferID());
+  StringRef macroDeclText =
+      SM.extractText(Lexer::getCharSourceRangeFromSourceRange(
+          SM, macro->getSourceRangeIncludingAttrs()));
+
   auto checkResult = swift_ASTGen_checkMacroDefinition(
-      &ctx.Diags, sourceFile->getExportedSourceFile(),
-      macro->getLoc().getOpaquePointerValue(), &externalMacroName,
-      &replacements, &numReplacements,
-      &genericReplacements, &numGenericReplacements);
+      &ctx.Diags, sourceFileText, macroDeclText, &externalMacroName,
+      &replacements, &numReplacements, &genericReplacements,
+      &numGenericReplacements);
 
   // Clean up after the call.
   SWIFT_DEFER {

test/Macros/lazy_parsing.swift

@@ -8,9 +8,6 @@
 // RUN: %empty-directory(%t/stats-lookup)
 // RUN: split-file %s %t
 
-// rdar://124548628 (additional ExportedSourceFileRequest(s) in `lazy_parsing.swift` (NCGenerics Enablement))
-// XFAIL: *
-
 // RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -parse-as-library -module-name=MacroDefinition %S/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath
 
 // RUN: %target-swift-frontend -typecheck -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) -stats-output-dir %t/stats-no-lookup -primary-file %t/b.swift %t/a.swift