Skip to content

Commit 03f4625

Browse files
redsun82redsun82
committed
Swift: go back to explicit DIAGNOSE_ERROR macros
1 parent 3f2a059 commit 03f4625

File tree

6 files changed

+113
-173
lines changed

6 files changed

+113
-173
lines changed

‎swift/integration-tests/diagnostics_test_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,5 @@ def check_diagnostics(test_dir=".", test_db="db"):
6060
actual_out.write(actual)
6161
actual = actual.splitlines(keepends=True)
6262
expected = expected.splitlines(keepends=True)
63-
print("".join(difflib.unified_diff(actual, expected, fromfile="diagnostics.actual", tofile="diagnostics.expected")), file=sys.stderr)
63+
print("".join(difflib.unified_diff(expected, actual, fromfile="diagnostics.expected", tofile="diagnostics.actual")), file=sys.stderr)
6464
sys.exit(1)

‎swift/logging/SwiftDiagnostics.cpp

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88

99
namespace codeql {
1010

11-
nlohmann::json SwiftDiagnosticsSource::json(const std::chrono::system_clock::time_point& timestamp,
12-
std::string_view message) const {
13-
return {
11+
nlohmann::json SwiftDiagnostic::json(const std::chrono::system_clock::time_point& timestamp,
12+
std::string_view message) const {
13+
nlohmann::json ret{
1414
{"source",
1515
{
16-
{"id", sourceId()},
16+
{"id", absl::StrJoin({extractorName, programName, id}, "/")},
1717
{"name", name},
1818
{"extractorName", extractorName},
1919
}},
@@ -28,26 +28,29 @@ nlohmann::json SwiftDiagnosticsSource::json(const std::chrono::system_clock::tim
2828
{"plaintextMessage", absl::StrCat(message, ".\n\n", action, ".")},
2929
{"timestamp", fmt::format("{:%FT%T%z}", timestamp)},
3030
};
31+
if (location) {
32+
ret["location"] = location->json();
33+
}
34+
return ret;
3135
}
3236

33-
std::string SwiftDiagnosticsSource::sourceId() const {
34-
return absl::StrJoin({extractorName, programName, id}, "/");
37+
std::string SwiftDiagnostic::abbreviation() const {
38+
if (location) {
39+
return absl::StrCat(id, "@", location->str());
40+
}
41+
return std::string{id};
3542
}
3643

37-
nlohmann::json SwiftDiagnosticsSourceWithLocation::json(
38-
const std::chrono::system_clock::time_point& timestamp,
39-
std::string_view message) const {
40-
auto ret = source.json(timestamp, message);
41-
auto& location = ret["location"] = {{"file", file}};
42-
if (startLine) location["startLine"] = startLine;
43-
if (startColumn) location["startColumn"] = startColumn;
44-
if (endLine) location["endLine"] = endLine;
45-
if (endColumn) location["endColumn"] = endColumn;
44+
nlohmann::json SwiftDiagnosticsLocation::json() const {
45+
nlohmann::json ret{{"file", file}};
46+
if (startLine) ret["startLine"] = startLine;
47+
if (startColumn) ret["startColumn"] = startColumn;
48+
if (endLine) ret["endLine"] = endLine;
49+
if (endColumn) ret["endColumn"] = endColumn;
4650
return ret;
4751
}
4852

49-
std::string SwiftDiagnosticsSourceWithLocation::str() const {
50-
return absl::StrCat(source.id, "@", file, ":", startLine, ":", startColumn, ":", endLine, ":",
51-
endColumn);
53+
std::string SwiftDiagnosticsLocation::str() const {
54+
return absl::StrJoin(std::tuple{file, startLine, startColumn, endLine, endColumn}, ":");
5255
}
5356
} // namespace codeql

‎swift/logging/SwiftDiagnostics.h

Lines changed: 18 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <string>
55
#include <vector>
66
#include <unordered_map>
7+
#include <optional>
78
#include <cassert>
89
#include <fstream>
910
#include <filesystem>
@@ -17,27 +18,22 @@ namespace codeql {
1718

1819
extern const std::string_view programName;
1920

20-
struct SwiftDiagnosticsSource;
21-
22-
struct SwiftDiagnosticsSourceWithLocation {
23-
const SwiftDiagnosticsSource& source;
21+
struct SwiftDiagnosticsLocation {
2422
std::string_view file;
2523
unsigned startLine;
2624
unsigned startColumn;
2725
unsigned endLine;
2826
unsigned endColumn;
2927

30-
// see SwiftDiagnosticsSource::json
31-
nlohmann::json json(const std::chrono::system_clock::time_point& timestamp,
32-
std::string_view message) const;
28+
nlohmann::json json() const;
3329

3430
std::string str() const;
3531
};
3632

3733
// Models a diagnostic source for Swift, holding static information that goes out into a diagnostic
3834
// These are internally stored into a map on id's. A specific error log can use binlog's category
3935
// as id, which will then be used to recover the diagnostic source while dumping.
40-
struct SwiftDiagnosticsSource {
36+
struct SwiftDiagnostic {
4137
std::string_view id;
4238
std::string_view name;
4339
static constexpr std::string_view extractorName = "swift";
@@ -46,6 +42,7 @@ struct SwiftDiagnosticsSource {
4642
// TODO(C++20) with vector going constexpr this can be turned to `std::vector<std::string_view>`
4743
std::string_view helpLinks;
4844

45+
std::optional<SwiftDiagnosticsLocation> location;
4946
// for the moment, we only output errors, so no need to store the severity
5047

5148
// create a JSON diagnostics for this source with the given timestamp and message to out
@@ -55,16 +52,18 @@ struct SwiftDiagnosticsSource {
5552
nlohmann::json json(const std::chrono::system_clock::time_point& timestamp,
5653
std::string_view message) const;
5754

58-
SwiftDiagnosticsSourceWithLocation withLocation(std::string_view file,
59-
unsigned startLine = 0,
60-
unsigned startColumn = 0,
61-
unsigned endLine = 0,
62-
unsigned endColumn = 0) const {
63-
return {*this, file, startLine, startColumn, endLine, endColumn};
55+
// returns <id> or <id>@<location> if a location is present
56+
std::string abbreviation() const;
57+
58+
SwiftDiagnostic withLocation(std::string_view file,
59+
unsigned startLine = 0,
60+
unsigned startColumn = 0,
61+
unsigned endLine = 0,
62+
unsigned endColumn = 0) const {
63+
auto ret = *this;
64+
ret.location = SwiftDiagnosticsLocation{file, startLine, startColumn, endLine, endColumn};
65+
return ret;
6466
}
65-
66-
private:
67-
std::string sourceId() const;
6867
};
6968

7069
class SwiftDiagnosticsDumper {
@@ -77,8 +76,7 @@ class SwiftDiagnosticsDumper {
7776

7877
void flush() { output.flush(); }
7978

80-
template <typename Source>
81-
void write(const Source& source,
79+
void write(const SwiftDiagnostic& source,
8280
const std::chrono::system_clock::time_point& timestamp,
8381
std::string_view message) {
8482
if (output) {
@@ -93,49 +91,9 @@ class SwiftDiagnosticsDumper {
9391
std::ofstream output;
9492
};
9593

96-
constexpr codeql::SwiftDiagnosticsSource internalError{
94+
constexpr SwiftDiagnostic internalError{
9795
"internal-error",
9896
"Internal error",
9997
"Contact us about this issue",
10098
};
10199
} // namespace codeql
102-
103-
namespace mserialize {
104-
// log diagnostic sources using just their id, using binlog/mserialize internal plumbing
105-
template <>
106-
struct CustomTag<codeql::SwiftDiagnosticsSource, void> : detail::BuiltinTag<std::string> {
107-
using T = codeql::SwiftDiagnosticsSource;
108-
};
109-
110-
template <>
111-
struct CustomSerializer<codeql::SwiftDiagnosticsSource, void> {
112-
template <typename OutputStream>
113-
static void serialize(const codeql::SwiftDiagnosticsSource& source, OutputStream& out) {
114-
mserialize::serialize(source.id, out);
115-
}
116-
117-
static size_t serialized_size(const codeql::SwiftDiagnosticsSource& source) {
118-
return mserialize::serialized_size(source.id);
119-
}
120-
};
121-
122-
template <>
123-
struct CustomTag<codeql::SwiftDiagnosticsSourceWithLocation, void>
124-
: detail::BuiltinTag<std::string> {
125-
using T = codeql::SwiftDiagnosticsSourceWithLocation;
126-
};
127-
128-
template <>
129-
struct CustomSerializer<codeql::SwiftDiagnosticsSourceWithLocation, void> {
130-
template <typename OutputStream>
131-
static void serialize(const codeql::SwiftDiagnosticsSourceWithLocation& source,
132-
OutputStream& out) {
133-
mserialize::serialize(source.str(), out);
134-
}
135-
136-
static size_t serialized_size(const codeql::SwiftDiagnosticsSourceWithLocation& source) {
137-
return mserialize::serialized_size(source.str());
138-
}
139-
};
140-
141-
} // namespace mserialize

‎swift/logging/SwiftLogging.h

Lines changed: 54 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,31 @@
3535
#define LOG_DEBUG(...) LOG_WITH_LEVEL(debug, __VA_ARGS__)
3636
#define LOG_TRACE(...) LOG_WITH_LEVEL(trace, __VA_ARGS__)
3737

38-
#define CODEQL_GET_SECOND(...) CODEQL_GET_SECOND_I(__VA_ARGS__, 0, 0)
39-
#define CODEQL_GET_SECOND_I(X, Y, ...) Y
40-
4138
// only do the actual logging if the picked up `Logger` instance is configured to handle the
4239
// provided log level. `LEVEL` must be a compile-time constant. `logger()` is evaluated once
43-
// TODO(C++20) replace non-standard ##__VA_ARGS__ with __VA_OPT__(,) __VA_ARGS__
44-
#define LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, CATEGORY, ...) \
45-
do { \
46-
static_assert(::codeql::detail::checkLogArgs<decltype(CODEQL_GET_SECOND(__VA_ARGS__))>( \
47-
MSERIALIZE_FIRST(__VA_ARGS__)), \
48-
"diagnostics logs must have format starting with \"[{}]\""); \
49-
constexpr auto _level = ::codeql::Log::Level::LEVEL; \
50-
::codeql::Logger& _logger = logger(); \
51-
if (_level >= _logger.level()) { \
52-
BINLOG_CREATE_SOURCE_AND_EVENT(_logger.writer(), _level, CATEGORY, ::binlog::clockNow(), \
53-
__VA_ARGS__); \
54-
} \
55-
if (_level >= ::codeql::Log::Level::error) { \
56-
::codeql::Log::flush(); \
57-
} \
40+
#define LOG_WITH_LEVEL(LEVEL, ...) \
41+
do { \
42+
constexpr auto _level = ::codeql::Log::Level::LEVEL; \
43+
::codeql::Logger& _logger = logger(); \
44+
if (_level >= _logger.level()) { \
45+
BINLOG_CREATE_SOURCE_AND_EVENT(_logger.writer(), _level, /*category*/, ::binlog::clockNow(), \
46+
__VA_ARGS__); \
47+
} \
48+
if (_level >= ::codeql::Log::Level::error) { \
49+
::codeql::Log::flush(); \
50+
} \
5851
} while (false)
5952

60-
#define LOG_WITH_LEVEL(LEVEL, ...) LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, , __VA_ARGS__)
53+
// Emit errors with a specified SwiftDiagnostic object. These will be both logged and outputted as
54+
// JSON DB diagnostics
55+
#define DIAGNOSE_CRITICAL(ID, ...) DIAGNOSE_WITH_LEVEL(critical, ID, __VA_ARGS__)
56+
#define DIAGNOSE_ERROR(ID, ...) DIAGNOSE_WITH_LEVEL(error, ID, __VA_ARGS__)
57+
58+
#define CODEQL_DIAGNOSTIC_LOG_FORMAT_PREFIX "[{}] "
59+
// TODO(C++20) replace non-standard , ##__VA_ARGS__ with __VA_OPT__(,) __VA_ARGS__
60+
#define DIAGNOSE_WITH_LEVEL(LEVEL, ID, FORMAT, ...) \
61+
LOG_WITH_LEVEL(LEVEL, CODEQL_DIAGNOSTIC_LOG_FORMAT_PREFIX FORMAT, \
62+
::codeql::detail::SwiftDiagnosticLogWrapper{ID}, ##__VA_ARGS__);
6163

6264
// avoid calling into binlog's original macros
6365
#undef BINLOG_CRITICAL
@@ -126,8 +128,7 @@ class Log {
126128
return instance().getLoggerConfigurationImpl(name);
127129
}
128130

129-
template <typename Source>
130-
static void diagnose(const Source& source,
131+
static void diagnose(const SwiftDiagnostic& source,
131132
const std::chrono::system_clock::time_point& time,
132133
std::string_view message) {
133134
instance().diagnostics.write(source, time, message);
@@ -224,31 +225,9 @@ class Logger {
224225
};
225226

226227
namespace detail {
227-
constexpr std::string_view diagnosticsFormatPrefix = "[{}] ";
228-
229-
template <typename T>
230-
constexpr bool checkLogArgs(std::string_view format) {
231-
using Type = std::remove_cv_t<std::remove_reference_t<T>>;
232-
constexpr bool isDiagnostic = std::is_same_v<Type, SwiftDiagnosticsSource> ||
233-
std::is_same_v<Type, SwiftDiagnosticsSourceWithLocation>;
234-
return !isDiagnostic ||
235-
format.substr(0, diagnosticsFormatPrefix.size()) == diagnosticsFormatPrefix;
236-
}
237-
238-
template <typename Writer, typename Source, typename... T>
239-
void binlogAddEventIgnoreFirstOverload(Writer& writer,
240-
std::uint64_t eventSourceId,
241-
std::uint64_t clock,
242-
const char* format,
243-
const Source& source,
244-
T&&... t) {
245-
std::chrono::system_clock::time_point point{
246-
std::chrono::duration_cast<std::chrono::system_clock::duration>(
247-
std::chrono::nanoseconds{clock})};
248-
constexpr auto offset = ::codeql::detail::diagnosticsFormatPrefix.size();
249-
::codeql::Log::diagnose(source, point, fmt::format(format + offset, t...));
250-
writer.addEvent(eventSourceId, clock, source, std::forward<T>(t)...);
251-
}
228+
struct SwiftDiagnosticLogWrapper {
229+
const SwiftDiagnostic& value;
230+
};
252231

253232
} // namespace detail
254233
} // namespace codeql
@@ -262,32 +241,37 @@ void addEventIgnoreFirst(Writer& writer,
262241
std::uint64_t eventSourceId,
263242
std::uint64_t clock,
264243
const char (&format)[N],
265-
const codeql::SwiftDiagnosticsSource& source,
244+
codeql::detail::SwiftDiagnosticLogWrapper&& source,
266245
T&&... t) {
267-
codeql::detail::binlogAddEventIgnoreFirstOverload(writer, eventSourceId, clock, format, source,
268-
std::forward<T>(t)...);
246+
std::chrono::system_clock::time_point point{
247+
std::chrono::duration_cast<std::chrono::system_clock::duration>(
248+
std::chrono::nanoseconds{clock})};
249+
constexpr std::string_view prefix = CODEQL_DIAGNOSTIC_LOG_FORMAT_PREFIX;
250+
::codeql::Log::diagnose(source.value, point, fmt::format(format + prefix.size(), t...));
251+
writer.addEvent(eventSourceId, clock, source, std::forward<T>(t)...);
269252
}
253+
} // namespace binlog::detail
270254

271-
template <typename Writer, size_t N, typename... T>
272-
void addEventIgnoreFirst(Writer& writer,
273-
std::uint64_t eventSourceId,
274-
std::uint64_t clock,
275-
const char (&format)[N],
276-
codeql::SwiftDiagnosticsSourceWithLocation&& source,
277-
T&&... t) {
278-
codeql::detail::binlogAddEventIgnoreFirstOverload(writer, eventSourceId, clock, format, source,
279-
std::forward<T>(t)...);
280-
}
255+
namespace mserialize {
256+
// log diagnostics wrapper using the abbreviation of the underlying diagnostic, using
257+
// binlog/mserialize internal plumbing
258+
template <>
259+
struct CustomTag<codeql::detail::SwiftDiagnosticLogWrapper, void>
260+
: detail::BuiltinTag<std::string> {
261+
using T = codeql::detail::SwiftDiagnosticLogWrapper;
262+
};
281263

282-
template <typename Writer, size_t N, typename... T>
283-
void addEventIgnoreFirst(Writer& writer,
284-
std::uint64_t eventSourceId,
285-
std::uint64_t clock,
286-
const char (&format)[N],
287-
const codeql::SwiftDiagnosticsSourceWithLocation& source,
288-
T&&... t) {
289-
codeql::detail::binlogAddEventIgnoreFirstOverload(writer, eventSourceId, clock, format, source,
290-
std::forward<T>(t)...);
291-
}
264+
template <>
265+
struct CustomSerializer<codeql::detail::SwiftDiagnosticLogWrapper, void> {
266+
template <typename OutputStream>
267+
static void serialize(const codeql::detail::SwiftDiagnosticLogWrapper& source,
268+
OutputStream& out) {
269+
mserialize::serialize(source.value.abbreviation(), out);
270+
}
292271

293-
} // namespace binlog::detail
272+
static size_t serialized_size(const codeql::detail::SwiftDiagnosticLogWrapper& source) {
273+
return mserialize::serialized_size(source.value.abbreviation());
274+
}
275+
};
276+
277+
} // namespace mserialize

‎swift/xcode-autobuilder/XcodeBuildRunner.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@
88
#include "swift/logging/SwiftLogging.h"
99
#include "swift/xcode-autobuilder/CustomizingBuildDiagnostics.h"
1010

11-
namespace codeql {
12-
constexpr SwiftDiagnosticsSource buildCommandFailed{
13-
"build-command-failed", "Detected build command failed", customizingBuildAction,
14-
customizingBuildHelpLinks};
15-
}
11+
constexpr codeql::SwiftDiagnostic buildCommandFailed{
12+
"build-command-failed", "Detected build command failed", codeql::customizingBuildAction,
13+
codeql::customizingBuildHelpLinks};
1614

1715
static codeql::Logger& logger() {
1816
static codeql::Logger ret{"build"};
@@ -70,8 +68,8 @@ void buildTarget(Target& target, bool dryRun) {
7068
std::cout << absl::StrJoin(argv, " ") << "\n";
7169
} else {
7270
if (!exec(argv)) {
73-
LOG_ERROR("[{}] The detected build command failed (tried {})", codeql::buildCommandFailed,
74-
absl::StrJoin(argv, " "));
71+
DIAGNOSE_ERROR(buildCommandFailed, "The detected build command failed (tried {})",
72+
absl::StrJoin(argv, " "));
7573
codeql::Log::flush();
7674
exit(1);
7775
}

0 commit comments

Comments
 (0)