Skip to content

[SwiftCompiler] Add DiagnosticEngine bridging #41500

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 1 commit into from
Feb 25, 2022
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
14 changes: 14 additions & 0 deletions SwiftCompilerSources/Sources/AST/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# This source file is part of the Swift.org open source project
#
# Copyright (c) 2022 Apple Inc. and the Swift project authors
# Licensed under Apache License v2.0 with Runtime Library Exception
#
# See http://swift.org/LICENSE.txt for license information
# See http://swift.org/CONTRIBUTORS.txt for Swift project authors

add_swift_compiler_module(AST
DEPENDS
Basic
SOURCES
DiagnosticEngine.swift)

123 changes: 123 additions & 0 deletions SwiftCompilerSources/Sources/AST/DiagnosticEngine.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
//===--- DiagnosticEngine.swift -------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2022 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import ASTBridging

import Basic

public typealias DiagID = BridgedDiagID

extension BridgedDiagnosticArgument {
init(stringRef val: BridgedStringRef) {
self.init(kind: .stringRef, value: .init(stringRefValue: val))
}
init(int val: Int) {
self.init(kind: .int, value: .init(intValue: val))
}
}

public protocol DiagnosticArgument {
func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void)
}
extension String: DiagnosticArgument {
public func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
withBridgedStringRef { fn(BridgedDiagnosticArgument(stringRef: $0)) }
}
}
extension Int: DiagnosticArgument {
public func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
fn(BridgedDiagnosticArgument(int: self))
}
}

public struct DiagnosticFixIt {
public let start: SourceLoc
public let byteLength: Int
public let text: String

public init(start: SourceLoc, byteLength: Int, replacement text: String) {
self.start = start
self.byteLength = byteLength
self.text = text
}

func withBridgedDiagnosticFixIt(_ fn: (BridgedDiagnosticFixIt) -> Void) {
text.withBridgedStringRef { bridgedTextRef in
let bridgedDiagnosticFixIt = BridgedDiagnosticFixIt(
start: start.bridged,
byteLength: byteLength,
text: bridgedTextRef)
fn(bridgedDiagnosticFixIt)
}
}
}

public struct DiagnosticEngine {
private let bridged: BridgedDiagnosticEngine

public init(bridged: BridgedDiagnosticEngine) {
self.bridged = bridged
}

public func diagnose(_ position: SourceLoc?,
_ id: DiagID,
_ args: [DiagnosticArgument],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this a var-arg parameter instead of an array?

Copy link
Member Author

@rintaro rintaro Feb 24, 2022

Choose a reason for hiding this comment

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

I added a var-arg variant, but I still want Array one too. It's convenient when, for example, using Swift's enum as an diagnostic type.
For example:

struct ParserDiagnostic {
  enum Message {
    case expectedNumDigits(String, Int)
    case miscError(String)
    case otherError
  }
  let location: Location
  let message: Message
}

func reportDiag(diag: ParseError) {
  let location = getSourceLoc(location)
  let diagID: DiagID
  let arguments: [DiagnosticArguement]

  switch diag.message {
  case expectedNumDigits(let s, let i):
    diagID = .parse_regex_expected_num_digits
    arguments = [s, I]

  case ...
    ...
  }
  
  engine.diagnose(loc, diagID, arguments)
}

Otherwise, we need to call DiagnositcEngine.diagnose() from each case

highlight: CharSourceRange? = nil,
fixIts: [DiagnosticFixIt] = []) {

let bridgedSourceLoc: BridgedSourceLoc = position.bridged
let bridgedHighlightRange: BridgedCharSourceRange = highlight.bridged
var bridgedArgs: [BridgedDiagnosticArgument] = []
var bridgedFixIts: [BridgedDiagnosticFixIt] = []

// Build a higher-order function to wrap every 'withBridgedXXX { ... }'
// calls, so we don't escape anything from the closure. 'bridgedArgs' and
// 'bridgedFixIts' are temporary storage to store bridged values. So they
// should not be used after the closue is executed.

var closure: () -> Void = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, this trick with the closure definitely needs a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added some comment

bridgedArgs.withBridgedArrayRef { bridgedArgsRef in
bridgedFixIts.withBridgedArrayRef { bridgedFixItsRef in
DiagnosticEngine_diagnose(bridged, bridgedSourceLoc,
id, bridgedArgsRef,
bridgedHighlightRange, bridgedFixItsRef)
}
}
}
for arg in args {
closure = { [closure, arg] in
arg._withBridgedDiagnosticArgument { bridgedArg in
bridgedArgs.append(bridgedArg)
closure()
}
}
}
for fixIt in fixIts {
closure = { [closure, fixIt] in
fixIt.withBridgedDiagnosticFixIt { bridgedFixIt in
bridgedFixIts.append(bridgedFixIt)
closure()
}
}
}

closure()
}

public func diagnose(_ position: SourceLoc?,
_ id: DiagID,
_ args: DiagnosticArgument...,
highlight: CharSourceRange? = nil,
fixIts: DiagnosticFixIt...) {
diagnose(position, id, args, highlight: highlight, fixIts: fixIts)
}
}
1 change: 1 addition & 0 deletions SwiftCompilerSources/Sources/Basic/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@
# See http://swift.org/CONTRIBUTORS.txt for Swift project authors

add_swift_compiler_module(Basic
SourceLoc.swift
Utils.swift)
67 changes: 67 additions & 0 deletions SwiftCompilerSources/Sources/Basic/SourceLoc.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//===--- SourceLoc.swift - SourceLoc bridiging utilities ------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2022 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

public struct SourceLoc {
Copy link
Contributor

Choose a reason for hiding this comment

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

I always found it confusing that we have multiple data types to describe a source location (in C++): SourceLoc and SILLocation.
I think we can do better on the swift side. We should unify AST.SourceLoc and SIL.Location and have only a single data type for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

SourceLoc and SILLocation are very different type. SourceLoc is basically just a pointer, whereas SILLocation is a much more complex object. I'm not sure how we could unify them...

/// Points into a source file.
let locationInFile: UnsafePointer<UInt8>

public init?(locationInFile: UnsafePointer<UInt8>?) {
guard let locationInFile = locationInFile else {
return nil
}
self.locationInFile = locationInFile
}

public init?(bridged: BridgedSourceLoc) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this shouldn't be a public API

Copy link
Member Author

Choose a reason for hiding this comment

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

In future, in other modules (i.e. Parse), we want to receiveBasic's Bridged* from C++
Like:

//-- Parse/Regex.swift
import Basic

// To be called from C++.
func parseRegexLiteral(_ bridgedLoc: BridgedSourceLoc, _ length: Int, ....) {
  let sourceLoc = SourceLoc(bridged: bridgedLoc)
  ...
}

So init(bridged:) needs to be public.

guard let locationInFile = bridged.pointer else {
return nil
}
self.init(locationInFile: locationInFile)
}

public var bridged: BridgedSourceLoc {
Copy link
Contributor

Choose a reason for hiding this comment

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

same 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.

This file is in Basic.
var bridged is used in DiagnosticEngine.diagnose() in AST to pass the bridged values to DiagnosticEngine_diagnose() C function.
I can make this underscored, but it'll be inconsistent with StringRef or ArrayRef's withBridgedXXX

.init(pointer: locationInFile)
}
}

extension Optional where Wrapped == SourceLoc {
public var bridged: BridgedSourceLoc {
self?.bridged ?? .init(pointer: nil)
}
}

public struct CharSourceRange {
private let start: SourceLoc
private let byteLength: Int

public init(start: SourceLoc, byteLength: Int) {
self.start = start
self.byteLength = byteLength
}

public init?(bridged: BridgedCharSourceRange) {
Copy link
Contributor

@eeckstein eeckstein Feb 23, 2022

Choose a reason for hiding this comment

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

This shouldn't be a public API

Copy link
Member Author

Choose a reason for hiding this comment

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

Same reason as SourceLoc

guard let start = SourceLoc(bridged: bridged.start) else {
return nil
}
self.init(start: start, byteLength: bridged.byteLength)
}

public var bridged: BridgedCharSourceRange {
Copy link
Contributor

Choose a reason for hiding this comment

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

same 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.

Same as SourceLoc

.init(start: start.bridged, byteLength: byteLength)
}
}

extension Optional where Wrapped == CharSourceRange {
public var bridged: BridgedCharSourceRange {
self?.bridged ?? .init(start: .init(pointer: nil), byteLength: 0)
}
}
1 change: 1 addition & 0 deletions SwiftCompilerSources/Sources/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# NOTE: Subdirectories must be added in dependency order.

add_subdirectory(Basic)
add_subdirectory(AST)
if(SWIFT_ENABLE_EXPERIMENTAL_STRING_PROCESSING)
add_subdirectory(ExperimentalRegex)
endif()
Expand Down
82 changes: 82 additions & 0 deletions include/swift/AST/ASTBridging.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
//===--- ASTBridging.h - header for the swift SILBridging module ----------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2022 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

#ifndef SWIFT_AST_ASTBRIDGING_H
#define SWIFT_AST_ASTBRIDGING_H

#include "swift/Basic/BasicBridging.h"
#include "swift/Basic/Compiler.h"
#include <stdbool.h>
#include <stddef.h>

#ifdef __cplusplus
extern "C" {
#endif

SWIFT_BEGIN_NULLABILITY_ANNOTATIONS

//===----------------------------------------------------------------------===//
// Diagnostic Engine
//===----------------------------------------------------------------------===//

// TODO: Move this to somewhere common header.
#if __has_attribute(enum_extensibility)
#define ENUM_EXTENSIBILITY_ATTR(arg) __attribute__((enum_extensibility(arg)))
#else
#define ENUM_EXTENSIBILITY_ATTR(arg)
#endif

// NOTE: This must be the same underlying value as C++ 'swift::DiagID' defined
// in 'DiagnosticList.cpp'.
typedef enum ENUM_EXTENSIBILITY_ATTR(open) BridgedDiagID : uint32_t {
#define DIAG(KIND, ID, Options, Text, Signature) BridgedDiagID_##ID,
#include "swift/AST/DiagnosticsAll.def"
} BridgedDiagID;

typedef enum ENUM_EXTENSIBILITY_ATTR(open) BridgedDiagnosticArgumentKind {
BridgedDiagnosticArgumentKind_StringRef,
BridgedDiagnosticArgumentKind_Int,
} BridgedDiagnosticArgumentKind;

typedef struct {
BridgedDiagnosticArgumentKind kind;
union {
BridgedStringRef stringRefValue;
SwiftInt intValue;
} value;
} BridgedDiagnosticArgument;

typedef struct {
BridgedSourceLoc start;
SwiftInt byteLength;
BridgedStringRef text;
} BridgedDiagnosticFixIt;

typedef struct {
void * _Nonnull object;
} BridgedDiagnosticEngine;

// FIXME: Can we bridge InFlightDiagnostic?
void DiagnosticEngine_diagnose(BridgedDiagnosticEngine, BridgedSourceLoc loc,
BridgedDiagID diagID, BridgedArrayRef arguments,
BridgedCharSourceRange highlight,
BridgedArrayRef fixIts);

bool DiagnosticEngine_hadAnyError(BridgedDiagnosticEngine);

SWIFT_END_NULLABILITY_ANNOTATIONS

#ifdef __cplusplus
} // extern "C"
#endif

#endif // SWIFT_AST_ASTBRIDGING_H
28 changes: 28 additions & 0 deletions include/swift/AST/BridgingUtils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//===--- BridgingUtils.h - utilities for swift bridging -------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2022 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

#ifndef SWIFT_AST_BRIDGINGUTILS_H
#define SWIFT_AST_BRIDGINGUTILS_H

#include "swift/AST/ASTBridging.h"
#include "swift/AST/DiagnosticEngine.h"

namespace swift {

inline BridgedDiagnosticEngine getBridgedDiagnosticEngine(DiagnosticEngine *D) {
return {(void *)D};
}

} // namespace swift

#endif

13 changes: 13 additions & 0 deletions include/swift/Basic/BasicBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,19 @@ void OStream_write(BridgedOStream os, BridgedStringRef str);

void freeBridgedStringRef(BridgedStringRef str);

//===----------------------------------------------------------------------===//
// Source location
//===----------------------------------------------------------------------===//

typedef struct {
const unsigned char * _Nullable pointer;
} BridgedSourceLoc;

typedef struct {
BridgedSourceLoc start;
SwiftInt byteLength;
} BridgedCharSourceRange;

SWIFT_END_NULLABILITY_ANNOTATIONS

#ifdef __cplusplus
Expand Down
21 changes: 21 additions & 0 deletions include/swift/Basic/BridgingUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,27 @@ inline llvm::ArrayRef<T> getArrayRef(BridgedArrayRef bridged) {
return {static_cast<const T *>(bridged.data), bridged.numElements};
}

inline SourceLoc getSourceLoc(const BridgedSourceLoc &bridged) {
return SourceLoc(
llvm::SMLoc::getFromPointer((const char *)bridged.pointer));
}

inline BridgedSourceLoc getBridgedSourceLoc(const SourceLoc &loc) {
return {static_cast<const unsigned char *>(loc.getOpaquePointerValue())};
}

inline CharSourceRange
getCharSourceRange(const BridgedCharSourceRange &bridged) {
auto start = getSourceLoc(bridged.start);
return CharSourceRange(start, bridged.byteLength);
}

inline BridgedCharSourceRange
getBridgedCharSourceRange(const CharSourceRange &range) {
auto start = getBridgedSourceLoc(range.getStart());
return {start, range.getByteLength()};
}

/// Copies the string in an malloc'ed memory and the caller is responsible for
/// freeing it. 'freeBridgedStringRef()' is available in 'BasicBridging.h'
inline BridgedStringRef
Expand Down
5 changes: 5 additions & 0 deletions include/swift/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ module BasicBridging {
export *
}

module ASTBridging {
header "AST/ASTBridging.h"
export *
}

module SILBridging {
header "SIL/SILBridging.h"
export *
Expand Down
Loading