Skip to content

[5.9] [SourceKit] Include generated macro buffers in diagnostic responses #65582

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 6 commits into from
May 3, 2023
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
3 changes: 1 addition & 2 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -1169,8 +1169,7 @@ namespace swift {

/// Retrieve the set of child notes that describe how the generated
/// source buffer was derived, e.g., a macro expansion backtrace.
std::vector<Diagnostic> getGeneratedSourceBufferNotes(
SourceLoc loc, Optional<unsigned> &lastBufferID);
std::vector<Diagnostic> getGeneratedSourceBufferNotes(SourceLoc loc);

/// Handle a new diagnostic, which will either be emitted, or added to an
/// active transaction.
Expand Down
18 changes: 7 additions & 11 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1272,9 +1272,8 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) {
diagnostic.isChildNote());
}

std::vector<Diagnostic> DiagnosticEngine::getGeneratedSourceBufferNotes(
SourceLoc loc, Optional<unsigned> &lastBufferID
) {
std::vector<Diagnostic>
DiagnosticEngine::getGeneratedSourceBufferNotes(SourceLoc loc) {
// The set of child notes we're building up.
std::vector<Diagnostic> childNotes;

Expand All @@ -1285,12 +1284,6 @@ std::vector<Diagnostic> DiagnosticEngine::getGeneratedSourceBufferNotes(
// If we already emitted these notes for a prior part of the diagnostic,
// don't do so again.
auto currentBufferID = SourceMgr.findBufferContainingLoc(loc);
if (currentBufferID == lastBufferID)
return childNotes;

// Keep track of the last buffer ID we considered.
lastBufferID = currentBufferID;

SourceLoc currentLoc = loc;
do {
auto generatedInfo = SourceMgr.getGeneratedSourceInfo(currentBufferID);
Expand Down Expand Up @@ -1337,15 +1330,18 @@ std::vector<Diagnostic> DiagnosticEngine::getGeneratedSourceBufferNotes(
}

void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {
Optional<unsigned> lastBufferID;

ArrayRef<Diagnostic> childNotes = diagnostic.getChildNotes();
std::vector<Diagnostic> extendedChildNotes;

if (auto info = diagnosticInfoForDiagnostic(diagnostic)) {
// If the diagnostic location is within a buffer containing generated
// source code, add child notes showing where the generation occurred.
extendedChildNotes = getGeneratedSourceBufferNotes(info->Loc, lastBufferID);
// We need to avoid doing this if this is itself a child note, as otherwise
// we'd end up doubling up on notes.
if (!info->IsChildNote) {
extendedChildNotes = getGeneratedSourceBufferNotes(info->Loc);
}
if (!extendedChildNotes.empty()) {
extendedChildNotes.insert(extendedChildNotes.end(),
childNotes.begin(), childNotes.end());
Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/SerializedDiagnosticConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ unsigned SerializedDiagnosticConsumer::getEmitFile(
// NOTE: Using Filename.data() here relies on SourceMgr using
// const char* as buffer identifiers. This is fast, but may
// be brittle. We can always switch over to using a StringMap.
// Note that the logic in EditorDiagConsumer::getBufferInfo
// will also need changing.
unsigned &existingEntry = State->Files[Filename.data()];
if (existingEntry)
return existingEntry;
Expand Down
9 changes: 9 additions & 0 deletions test/Macros/Inputs/syntax_macro_definitions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,15 @@ public struct InvalidMacro: PeerMacro, DeclarationMacro {
}
}

public struct CoerceToIntMacro: ExpressionMacro {
public static func expansion(
of node: some FreestandingMacroExpansionSyntax,
in context: some MacroExpansionContext
) -> ExprSyntax {
"\(node.argumentList.first!.expression) as Int"
}
}

public struct WrapInType: PeerMacro {
public static func expansion(
of node: AttributeSyntax,
Expand Down
2 changes: 1 addition & 1 deletion test/Macros/macro_expand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func testDiscardableStringify(x: Int) {
func testNested() {
struct Nested { }
_ = #stringify(#assertAny(Nested()))
// expected-note@-1 2 {{in expansion of macro 'stringify' here}}
// expected-note@-1 {{in expansion of macro 'stringify' here}}
// CHECK-DIAGS-NOT: error: cannot convert value of type 'Nested' to expected argument type 'Bool'
// CHECK-DIAGS: @__swiftmacro_9MacroUser10testNestedyyF9stringifyfMf_9assertAnyfMf_.swift:1:8: error: cannot convert value of type 'Nested' to expected argument type 'Bool'
// CHECK-DIAGS-NOT: error: cannot convert value of type 'Nested' to expected argument type 'Bool'
Expand Down
38 changes: 38 additions & 0 deletions test/Macros/macro_expand_other.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Expanding macros that are defined in terms of other macros.

// RUN: %empty-directory(%t)
// 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

// Diagnostics testing
// RUN: %target-typecheck-verify-swift -swift-version 5 -enable-experimental-feature FreestandingMacros -load-plugin-library %t/%target-library-name(MacroDefinition) -I %swift-host-lib-dir -module-name MacroUser -DTEST_DIAGNOSTICS

// Execution testing
// RUN: %target-build-swift -swift-version 5 -enable-experimental-feature FreestandingMacros -load-plugin-library %t/%target-library-name(MacroDefinition) -I %swift-host-lib-dir -L %swift-host-lib-dir %s -o %t/main -module-name MacroUser
// RUN: %target-codesign %t/main
// RUN: %target-run %t/main | %FileCheck %s
// REQUIRES: swift_swift_parser, executable_test

@freestanding(expression) macro stringify<T>(_ value: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro")

@freestanding(expression) macro stringifySeven() -> (Int, String) = #stringify(7)

@freestanding(expression) macro recurse(_: Bool) = #externalMacro(module: "MacroDefinition", type: "RecursiveMacro")

@freestanding(expression) macro recurseThrough(_ value: Bool) = #recurse(value)

func testFreestandingExpansionOfOther() {
// CHECK: ---testFreestandingExpansionOfOther
print("---testFreestandingExpansionOfOther")

// CHECK-NEXT: (7, "7")
print(#stringifySeven)

#recurseThrough(false)

#if TEST_DIAGNOSTICS
#recurseThrough(true)
// expected-note@-1 {{in expansion of macro 'recurseThrough' here}}
#endif
}

testFreestandingExpansionOfOther()
44 changes: 44 additions & 0 deletions test/SourceKit/Diagnostics/diag_in_c_header.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t
// RUN: %sourcekitd-test -req=diags %t/main.swift -- %t/main.swift -I %t | %FileCheck %s

//--- Header.h
#define FOO(x) #x

//--- module.modulemap
module HeaderWithMacro {
header "Header.h"
}

//--- main.swift
import HeaderWithMacro

_ = FOO(5)

// rdar://107281079 – Make sure the note points in the .h file
// CHECK: key.diagnostics: [
// CHECK-NEXT: {
// CHECK-NEXT: key.line: 3,
// CHECK-NEXT: key.column: 5,
// CHECK-NEXT: key.filepath: "{{.*}}main.swift",
// CHECK-NEXT: key.severity: source.diagnostic.severity.error,
// CHECK-NEXT: key.id: "cannot_find_in_scope",
// CHECK-NEXT: key.description: "cannot find 'FOO' in scope",
// CHECK-NEXT: key.ranges: [
// CHECK-NEXT: {
// CHECK-NEXT: key.offset: 28,
// CHECK-NEXT: key.length: 3
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: key.diagnostics: [
// CHECK-NEXT: {
// CHECK-NEXT: key.line: 1,
// CHECK-NEXT: key.column: 9,
// CHECK-NEXT: key.filepath: "{{.*}}Header.h",
// CHECK-NEXT: key.severity: source.diagnostic.severity.note,
// CHECK-NEXT: key.id: "macro_not_imported_function_like",
// CHECK-NEXT: key.description: "macro 'FOO' unavailable: function like macros not supported"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: }
// CHECK-NEXT: ]
38 changes: 26 additions & 12 deletions test/SourceKit/Inputs/sourcekitd_path_sanitize.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,35 @@
import re
import sys

SWIFTMODULE_BUNDLE_RE = re.compile(
r'key.filepath: ".*[/\\](.*)\.swiftmodule[/\\].*\.swiftmodule"')
SWIFTMODULE_RE = re.compile(r'key.filepath: ".*[/\\](.*)\.swiftmodule"')
SWIFT_RE = re.compile(r'key.filepath: ".*[/\\](.*)\.swift"')
PCM_RE = re.compile(r'key.filepath: ".*[/\\](.*)-[0-9A-Z]*\.pcm"')
HEADER_RE = re.compile(r' file=\\".*[/\\](.*)\.h\\"')
# I'm sorry dear reader, unfortunately my knowledge of regex trumps my knowledge of
# Python. This pattern allows us to clean up file paths by stripping them down to
# just the relevant file name.
RE = re.compile(
# The key can either be 'filepath' or 'buffer_name'. Also apply this logic to
# the file name in XML, which is written 'file=\"...\"'.
r'(key\.(?:filepath|buffer_name): |file=)'

# Open delimiter with optional escape.
r'\\?"'

# Lazily match characters until we hit a slash, then match any non-slash that
# ends in the right file extension, capturing the result.
r'.*?[/\\]+([^/\\]*\.(?:swiftmodule|swift|pcm|h))'

# For swiftmodule bundles, we want to match against the directory name, so
# optionally match the .swiftmodule filename here. The lazy matching of the
# previous logic means we'll prefer to match the previous '.swiftmodule' as the
# directory.
r'(?:[/\\]+[^/\\]*\.swiftmodule)?'

# Close delimiter with optional escape.
r'\\?"'
)

try:
for line in sys.stdin.readlines():
line = re.sub(SWIFTMODULE_BUNDLE_RE,
r'key.filepath: \1.swiftmodule', line)
line = re.sub(SWIFTMODULE_RE, r'key.filepath: \1.swiftmodule', line)
line = re.sub(SWIFT_RE, r'key.filepath: \1.swift', line)
line = re.sub(PCM_RE, r'key.filepath: \1.pcm', line)
line = re.sub(HEADER_RE, r' file=\1.h', line)
# We substitute in both the key and the matched filename.
line = re.sub(RE, r'\1\2', line)
sys.stdout.write(line)
except KeyboardInterrupt:
sys.stdout.flush()
27 changes: 27 additions & 0 deletions test/SourceKit/Macros/diags.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
@freestanding(expression)
macro coerceToInt<T>(_: T) -> Int = #externalMacro(module: "MacroDefinition", type: "CoerceToIntMacro")

@freestanding(expression)
macro stringify<T>(_: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro")

@attached(peer)
macro Invalid() = #externalMacro(module: "MacroDefinition", type: "InvalidMacro")

func foo() {
let _ = #coerceToInt("a")
let _ = #coerceToInt("b")
let _ = #stringify(#coerceToInt("c"))
}

@Invalid
struct Bad {}

// REQUIRES: swift_swift_parser

// RUN: %empty-directory(%t)

//##-- Prepare the macro plugin.
// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/../../Macros/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath

// RUN: %sourcekitd-test -req=diags %s -- -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) -module-name MacroUser %s | %sed_clean > %t.response
// RUN: %diff -u %s.response %t.response
Loading