From c7c12a7108710cbfcd4b27c100c5345d61cae9c4 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Tue, 2 May 2023 17:54:07 +0200 Subject: [PATCH 01/10] Swift: add json and date dependencies --- misc/bazel/workspace.bzl | 10 ---- swift/extractor/infra/log/BUILD.bazel | 5 +- .../{binlog => }/BUILD.binlog.bazel | 0 swift/third_party/BUILD.date.bazel | 6 +++ .../{picosha2 => }/BUILD.picosha2.bazel | 0 .../BUILD.swift-llvm-support.bazel | 0 swift/third_party/binlog/BUILD.bazel | 0 swift/third_party/load.bzl | 53 ++++++++++++------- swift/third_party/picosha2/BUILD.bazel | 0 9 files changed, 43 insertions(+), 31 deletions(-) rename swift/third_party/{binlog => }/BUILD.binlog.bazel (100%) create mode 100644 swift/third_party/BUILD.date.bazel rename swift/third_party/{picosha2 => }/BUILD.picosha2.bazel (100%) rename swift/third_party/{swift-llvm-support => }/BUILD.swift-llvm-support.bazel (100%) delete mode 100644 swift/third_party/binlog/BUILD.bazel delete mode 100644 swift/third_party/picosha2/BUILD.bazel diff --git a/misc/bazel/workspace.bzl b/misc/bazel/workspace.bzl index 50f0d1393f0e..ef89ccfb666e 100644 --- a/misc/bazel/workspace.bzl +++ b/misc/bazel/workspace.bzl @@ -43,13 +43,3 @@ def codeql_workspace(repository_name = "codeql"): "https://github.com/bazelbuild/bazel-skylib/releases/download/1.4.1/bazel-skylib-1.4.1.tar.gz", ], ) - - maybe( - repo_rule = http_archive, - name = "absl", - sha256 = "cec2e5bf780532bd0ac672eb8d43c0f8bbe84ca5df8718320184034b7f59a398", - urls = [ - "https://github.com/abseil/abseil-cpp/archive/d2c5297a3c3948de765100cb7e5cccca1210d23c.tar.gz", - ], - strip_prefix = "abseil-cpp-d2c5297a3c3948de765100cb7e5cccca1210d23c", - ) diff --git a/swift/extractor/infra/log/BUILD.bazel b/swift/extractor/infra/log/BUILD.bazel index 2a384e136a5a..6edc8f795f58 100644 --- a/swift/extractor/infra/log/BUILD.bazel +++ b/swift/extractor/infra/log/BUILD.bazel @@ -3,5 +3,8 @@ cc_library( srcs = glob(["*.cpp"]), hdrs = glob(["*.h"]), visibility = ["//visibility:public"], - deps = ["@binlog"], + deps = [ + "@binlog", + "@json", + ], ) diff --git a/swift/third_party/binlog/BUILD.binlog.bazel b/swift/third_party/BUILD.binlog.bazel similarity index 100% rename from swift/third_party/binlog/BUILD.binlog.bazel rename to swift/third_party/BUILD.binlog.bazel diff --git a/swift/third_party/BUILD.date.bazel b/swift/third_party/BUILD.date.bazel new file mode 100644 index 000000000000..598d9efea3b2 --- /dev/null +++ b/swift/third_party/BUILD.date.bazel @@ -0,0 +1,6 @@ +cc_library( + name = "date", + hdrs = glob(["include/date/*.h"]), + includes = ["include"], + visibility = ["//visibility:public"], +) diff --git a/swift/third_party/picosha2/BUILD.picosha2.bazel b/swift/third_party/BUILD.picosha2.bazel similarity index 100% rename from swift/third_party/picosha2/BUILD.picosha2.bazel rename to swift/third_party/BUILD.picosha2.bazel diff --git a/swift/third_party/swift-llvm-support/BUILD.swift-llvm-support.bazel b/swift/third_party/BUILD.swift-llvm-support.bazel similarity index 100% rename from swift/third_party/swift-llvm-support/BUILD.swift-llvm-support.bazel rename to swift/third_party/BUILD.swift-llvm-support.bazel diff --git a/swift/third_party/binlog/BUILD.bazel b/swift/third_party/binlog/BUILD.bazel deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/swift/third_party/load.bzl b/swift/third_party/load.bzl index 643fad096dbb..a85ede792e37 100644 --- a/swift/third_party/load.bzl +++ b/swift/third_party/load.bzl @@ -1,4 +1,5 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") _swift_prebuilt_version = "swift-5.7.3-RELEASE.142" _swift_sha_map = { @@ -12,28 +13,20 @@ _swift_arch_map = { "macOS-X64": "darwin_x86_64", } -def _get_label(workspace_name, package, target): - return "@%s//swift/third_party/%s:%s" % (workspace_name, package, target) - -def _get_build(workspace_name, package): - return _get_label(workspace_name, package, "BUILD.%s.bazel" % package) - -def _get_patch(workspace_name, package, patch): - return _get_label(workspace_name, package, "patches/%s.patch" % patch) - -def _github_archive(*, name, workspace_name, repository, commit, sha256 = None, patches = None): +def _github_archive(*, name, repository, commit, build_file = None, sha256 = None): github_name = repository[repository.index("/") + 1:] - patches = [_get_patch(workspace_name, name, p) for p in patches or []] - http_archive( + maybe( + repo_rule = http_archive, name = name, url = "https://github.com/%s/archive/%s.zip" % (repository, commit), strip_prefix = "%s-%s" % (github_name, commit), - build_file = _get_build(workspace_name, name), + build_file = build_file, sha256 = sha256, - patch_args = ["-p1"], - patches = patches, ) +def _build(workspace_name, package): + return "@%s//swift/third_party:BUILD.%s.bazel" % (workspace_name, package) + def load_dependencies(workspace_name): for repo_arch, arch in _swift_arch_map.items(): sha256 = _swift_sha_map[repo_arch] @@ -44,15 +37,13 @@ def load_dependencies(workspace_name): _swift_prebuilt_version, repo_arch, ), - build_file = _get_build(workspace_name, "swift-llvm-support"), + build_file = _build(workspace_name, "swift-llvm-support"), sha256 = sha256, - patch_args = ["-p1"], - patches = [], ) _github_archive( name = "picosha2", - workspace_name = workspace_name, + build_file = _build(workspace_name, "picosha2"), repository = "okdshin/PicoSHA2", commit = "27fcf6979298949e8a462e16d09a0351c18fcaf2", sha256 = "d6647ca45a8b7bdaf027ecb68d041b22a899a0218b7206dee755c558a2725abb", @@ -60,8 +51,30 @@ def load_dependencies(workspace_name): _github_archive( name = "binlog", - workspace_name = workspace_name, + build_file = _build(workspace_name, "binlog"), repository = "morganstanley/binlog", commit = "3fef8846f5ef98e64211e7982c2ead67e0b185a6", sha256 = "f5c61d90a6eff341bf91771f2f465be391fd85397023e1b391c17214f9cbd045", ) + + _github_archive( + name = "absl", + repository = "abseil/abseil-cpp", + commit = "d2c5297a3c3948de765100cb7e5cccca1210d23c", + sha256 = "735a9efc673f30b3212bfd57f38d5deb152b543e35cd58b412d1363b15242049", + ) + + _github_archive( + name = "json", + repository = "nlohmann/json", + commit = "6af826d0bdb55e4b69e3ad817576745335f243ca", + sha256 = "702bb0231a5e21c0374230fed86c8ae3d07ee50f34ffd420e7f8249854b7d85b", + ) + + _github_archive( + name = "date", + build_file = _build(workspace_name, "date"), + repository = "HowardHinnant/date", + commit = "6e921e1b1d21e84a5c82416ba7ecd98e33a436d0", + sha256 = "484c450ea1cec479716f7cfce9a54da1867dd4043dde08e7c262b812561fe3bc", + ) diff --git a/swift/third_party/picosha2/BUILD.bazel b/swift/third_party/picosha2/BUILD.bazel deleted file mode 100644 index e69de29bb2d1..000000000000 From 0ad529dff82ad6c575ddf7e1484523bd83479f22 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Wed, 3 May 2023 05:46:12 +0200 Subject: [PATCH 02/10] Swift: move logging to a common directory --- swift/extractor/infra/BUILD.bazel | 2 +- swift/extractor/infra/SwiftDispatcher.h | 2 +- swift/extractor/main.cpp | 2 +- swift/extractor/trap/BUILD.bazel | 7 +++++-- swift/extractor/trap/TrapDomain.h | 2 +- swift/{extractor/infra => }/log/BUILD.bazel | 0 swift/{extractor/infra => }/log/SwiftLogging.cpp | 2 +- swift/{extractor/infra => }/log/SwiftLogging.h | 0 8 files changed, 10 insertions(+), 7 deletions(-) rename swift/{extractor/infra => }/log/BUILD.bazel (100%) rename swift/{extractor/infra => }/log/SwiftLogging.cpp (99%) rename swift/{extractor/infra => }/log/SwiftLogging.h (100%) diff --git a/swift/extractor/infra/BUILD.bazel b/swift/extractor/infra/BUILD.bazel index 0b69dc1c3b9c..1d1b94a759cd 100644 --- a/swift/extractor/infra/BUILD.bazel +++ b/swift/extractor/infra/BUILD.bazel @@ -8,8 +8,8 @@ swift_cc_library( deps = [ "//swift/extractor/config", "//swift/extractor/infra/file", - "//swift/extractor/infra/log", "//swift/extractor/trap", + "//swift/log", "//swift/third_party/swift-llvm-support", ], ) diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h index 6ab49ec79f80..b0bbc793f660 100644 --- a/swift/extractor/infra/SwiftDispatcher.h +++ b/swift/extractor/infra/SwiftDispatcher.h @@ -13,7 +13,7 @@ #include "swift/extractor/infra/SwiftBodyEmissionStrategy.h" #include "swift/extractor/infra/SwiftMangledName.h" #include "swift/extractor/config/SwiftExtractorState.h" -#include "swift/extractor/infra/log/SwiftLogging.h" +#include "swift/log/SwiftLogging.h" namespace codeql { diff --git a/swift/extractor/main.cpp b/swift/extractor/main.cpp index 200868c9d986..a85c93b1ee8a 100644 --- a/swift/extractor/main.cpp +++ b/swift/extractor/main.cpp @@ -18,7 +18,7 @@ #include "swift/extractor/invocation/SwiftInvocationExtractor.h" #include "swift/extractor/trap/TrapDomain.h" #include "swift/extractor/infra/file/Path.h" -#include "swift/extractor/infra/log/SwiftLogging.h" +#include "swift/log/SwiftLogging.h" using namespace std::string_literals; diff --git a/swift/extractor/trap/BUILD.bazel b/swift/extractor/trap/BUILD.bazel index 59870efbf883..8ac63aba0855 100644 --- a/swift/extractor/trap/BUILD.bazel +++ b/swift/extractor/trap/BUILD.bazel @@ -23,7 +23,10 @@ genrule( "--schema=$(location //swift:schema)", "--script-name=codegen/codegen.py", ]), - exec_tools = ["//misc/codegen", "//swift:schema"], + exec_tools = [ + "//misc/codegen", + "//swift:schema", + ], ) filegroup( @@ -49,7 +52,7 @@ swift_cc_library( visibility = ["//visibility:public"], deps = [ "//swift/extractor/infra/file", - "//swift/extractor/infra/log", + "//swift/log", "@absl//absl/numeric:bits", ], ) diff --git a/swift/extractor/trap/TrapDomain.h b/swift/extractor/trap/TrapDomain.h index 6709873f5bea..a9b2ab672c00 100644 --- a/swift/extractor/trap/TrapDomain.h +++ b/swift/extractor/trap/TrapDomain.h @@ -5,7 +5,7 @@ #include "swift/extractor/trap/TrapLabel.h" #include "swift/extractor/infra/file/TargetFile.h" -#include "swift/extractor/infra/log/SwiftLogging.h" +#include "swift/log/SwiftLogging.h" #include "swift/extractor/infra/SwiftMangledName.h" namespace codeql { diff --git a/swift/extractor/infra/log/BUILD.bazel b/swift/log/BUILD.bazel similarity index 100% rename from swift/extractor/infra/log/BUILD.bazel rename to swift/log/BUILD.bazel diff --git a/swift/extractor/infra/log/SwiftLogging.cpp b/swift/log/SwiftLogging.cpp similarity index 99% rename from swift/extractor/infra/log/SwiftLogging.cpp rename to swift/log/SwiftLogging.cpp index ab7ae278d4d8..bfed24ef55e4 100644 --- a/swift/extractor/infra/log/SwiftLogging.cpp +++ b/swift/log/SwiftLogging.cpp @@ -1,4 +1,4 @@ -#include "swift/extractor/infra/log/SwiftLogging.h" +#include "swift/log/SwiftLogging.h" #include #include diff --git a/swift/extractor/infra/log/SwiftLogging.h b/swift/log/SwiftLogging.h similarity index 100% rename from swift/extractor/infra/log/SwiftLogging.h rename to swift/log/SwiftLogging.h From 8de2f9958e7322baec0214dcd093b9c2618d268c Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Wed, 3 May 2023 12:01:50 +0200 Subject: [PATCH 03/10] Swift: add support to output JSON diagnostics New `DIAGNOSE_ERROR` and `DIAGNOSE_CRITICAL` macros are added. These accept an ID which should indicate a diagnostic source via a function definition in `codeql::diagnostics`, together with the usual format + arguments accepted by other `LOG_*` macros. When the log is flushed, these special logs will result in an error JSON diagnostic entry in the database. --- swift/extractor/main.cpp | 3 +- swift/log/BUILD.bazel | 1 + swift/log/SwiftDiagnostics.cpp | 73 +++++++++++++++++++++++++++++ swift/log/SwiftDiagnostics.h | 86 ++++++++++++++++++++++++++++++++++ swift/log/SwiftLogging.cpp | 31 ++++++++++-- swift/log/SwiftLogging.h | 78 +++++++++++++++++++++--------- 6 files changed, 245 insertions(+), 27 deletions(-) create mode 100644 swift/log/SwiftDiagnostics.cpp create mode 100644 swift/log/SwiftDiagnostics.h diff --git a/swift/extractor/main.cpp b/swift/extractor/main.cpp index a85c93b1ee8a..a9159e034ec1 100644 --- a/swift/extractor/main.cpp +++ b/swift/extractor/main.cpp @@ -22,7 +22,7 @@ using namespace std::string_literals; -const std::string_view codeql::logRootName = "extractor"; +const std::string_view codeql::programName = "extractor"; // must be called before processFrontendOptions modifies output paths static void lockOutputSwiftModuleTraps(codeql::SwiftExtractorState& state, @@ -220,6 +220,7 @@ int main(int argc, char** argv, char** envp) { codeql::Logger logger{"main"}; LOG_INFO("calling extractor with arguments \"{}\"", argDump(argc, argv)); LOG_DEBUG("environment:\n{}\n", envDump(envp)); + DIAGNOSE_ERROR(internal_error, "prout {}", 42); } auto openInterception = codeql::setupFileInterception(configuration); diff --git a/swift/log/BUILD.bazel b/swift/log/BUILD.bazel index 6edc8f795f58..a2532ea75f1d 100644 --- a/swift/log/BUILD.bazel +++ b/swift/log/BUILD.bazel @@ -5,6 +5,7 @@ cc_library( visibility = ["//visibility:public"], deps = [ "@binlog", + "@date", "@json", ], ) diff --git a/swift/log/SwiftDiagnostics.cpp b/swift/log/SwiftDiagnostics.cpp new file mode 100644 index 000000000000..ce29d0c57d61 --- /dev/null +++ b/swift/log/SwiftDiagnostics.cpp @@ -0,0 +1,73 @@ +#include "swift/log/SwiftDiagnostics.h" + +#include +#include +#include + +namespace codeql { +SwiftDiagnosticsSource::SwiftDiagnosticsSource(std::string_view internalId, + std::string&& name, + std::vector&& helpLinks, + std::string&& action) + : name{std::move(name)}, helpLinks{std::move(helpLinks)}, action{std::move(action)} { + id = extractorName; + id += '/'; + id += programName; + id += '/'; + std::transform(internalId.begin(), internalId.end(), std::back_inserter(id), + [](char c) { return c == '_' ? '-' : c; }); +} + +void SwiftDiagnosticsSource::create(std::string_view id, + std::string name, + std::vector helpLinks, + std::string action) { + auto [it, inserted] = map().emplace( + id, SwiftDiagnosticsSource{id, std::move(name), std::move(helpLinks), std::move(action)}); + assert(inserted); +} + +void SwiftDiagnosticsSource::emit(std::ostream& out, + std::string_view timestamp, + std::string_view message) const { + nlohmann::json entry; + auto& source = entry["source"]; + source["id"] = id; + source["name"] = name; + source["extractorName"] = extractorName; + + auto& visibility = entry["visibility"]; + visibility["statusPage"] = true; + visibility["cliSummaryTable"] = true; + visibility["telemetry"] = true; + + entry["severity"] = "error"; + entry["helpLinks"] = helpLinks; + std::string plaintextMessage{message}; + plaintextMessage += ".\n\n"; + plaintextMessage += action; + plaintextMessage += '.'; + entry["plaintextMessage"] = plaintextMessage; + + entry["timestamp"] = timestamp; + + out << entry << '\n'; +} + +void SwiftDiagnosticsDumper::write(const char* buffer, std::size_t bufferSize) { + binlog::Range range{buffer, bufferSize}; + binlog::RangeEntryStream input{range}; + while (auto event = events.nextEvent(input)) { + const auto& source = SwiftDiagnosticsSource::get(event->source->category); + std::ostringstream oss; + timestampedMessagePrinter.printEvent(oss, *event, events.writerProp(), events.clockSync()); + auto data = oss.str(); + std::string_view view = data; + auto sep = view.find(' '); + assert(sep != std::string::npos); + auto timestamp = view.substr(0, sep); + auto message = view.substr(sep + 1); + source.emit(output, timestamp, message); + } +} +} // namespace codeql diff --git a/swift/log/SwiftDiagnostics.h b/swift/log/SwiftDiagnostics.h new file mode 100644 index 000000000000..5e0953761168 --- /dev/null +++ b/swift/log/SwiftDiagnostics.h @@ -0,0 +1,86 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace codeql { + +extern const std::string_view programName; + +// Models a diagnostic source for Swift, holding static information that goes out into a diagnostic +// These are internally stored into a map on id's. A specific error log can use binlog's category +// as id, which will then be used to recover the diagnostic source while dumping. +class SwiftDiagnosticsSource { + public: + // creates a SwiftDiagnosticsSource with the given data + static void create(std::string_view id, + std::string name, + std::vector helpLinks, + std::string action); + + // gets a previously created SwiftDiagnosticsSource for the given id. Will abort if none exists + static const SwiftDiagnosticsSource& get(const std::string& id) { return map().at(id); } + + // emit a JSON diagnostics for this source with the given timestamp and message to out + // A plaintextMessage is used that includes both the message and the action to take. Dots are + // appended to both. The id is used to construct the source id in the form + // `swift//` + void emit(std::ostream& out, std::string_view timestamp, std::string_view message) const; + + private: + using Map = std::unordered_map; + + std::string id; + std::string name; + static constexpr std::string_view extractorName = "swift"; + + // for the moment, we only output errors, so no need to store the severity + + std::vector helpLinks; + std::string action; + + static Map& map() { + static Map ret; + return ret; + } + + SwiftDiagnosticsSource(std::string_view internalId, + std::string&& name, + std::vector&& helpLinks, + std::string&& action); +}; + +// An output modeling binlog's output stream concept that intercepts binlog entries and translates +// them to appropriate diagnostics JSON entries +class SwiftDiagnosticsDumper { + public: + // opens path for writing out JSON entries. Returns whether the operation was successful. + bool open(const std::filesystem::path& path) { + output.open(path); + return output.good(); + } + + // write out binlog entries as corresponding JSON diagnostics entries. Expects all entries to have + // a category equal to an id of a previously created SwiftDiagnosticSource. + void write(const char* buffer, std::size_t bufferSize); + + private: + binlog::EventStream events; + std::ofstream output; + binlog::PrettyPrinter timestampedMessagePrinter{"%u %m", "%Y-%m-%dT%H:%M:%S.%NZ"}; +}; + +namespace diagnostics { +inline void internal_error() { + SwiftDiagnosticsSource::create("internal_error", "Internal error", {}, + "Contact us about this issue"); +} +} // namespace diagnostics + +} // namespace codeql diff --git a/swift/log/SwiftLogging.cpp b/swift/log/SwiftLogging.cpp index bfed24ef55e4..c96897da398d 100644 --- a/swift/log/SwiftLogging.cpp +++ b/swift/log/SwiftLogging.cpp @@ -50,7 +50,7 @@ Log::Level matchToLevel(std::csub_match m) { } // namespace -std::vector Log::collectSeverityRulesAndReturnProblems(const char* envVar) { +std::vector Log::collectLevelRulesAndReturnProblems(const char* envVar) { std::vector problems; if (auto levels = getEnvOr(envVar, nullptr)) { // expect comma-separated : @@ -92,12 +92,13 @@ std::vector Log::collectSeverityRulesAndReturnProblems(const char* void Log::configure() { // as we are configuring logging right now, we collect problems and log them at the end - auto problems = collectSeverityRulesAndReturnProblems("CODEQL_EXTRACTOR_SWIFT_LOG_LEVELS"); + auto problems = collectLevelRulesAndReturnProblems("CODEQL_EXTRACTOR_SWIFT_LOG_LEVELS"); + auto now = std::to_string(std::chrono::system_clock::now().time_since_epoch().count()); if (text || binary) { std::filesystem::path logFile = getEnvOr("CODEQL_EXTRACTOR_SWIFT_LOG_DIR", "extractor-out/log"); logFile /= "swift"; - logFile /= logRootName; - logFile /= std::to_string(std::chrono::system_clock::now().time_since_epoch().count()); + logFile /= programName; + logFile /= now; std::error_code ec; std::filesystem::create_directories(logFile.parent_path(), ec); if (!ec) { @@ -123,6 +124,25 @@ void Log::configure() { binary.level = Level::no_logs; text.level = Level::no_logs; } + if (diagnostics) { + std::filesystem::path diagFile = + getEnvOr("CODEQL_EXTRACTOR_SWIFT_DIAGNOSTIC_DIR", "extractor-out/diagnostics"); + diagFile /= programName; + diagFile /= now; + diagFile.replace_extension(".jsonl"); + std::error_code ec; + std::filesystem::create_directories(diagFile.parent_path(), ec); + if (!ec) { + if (!diagnostics.output.open(diagFile)) { + problems.emplace_back("Unable to open diagnostics json file " + diagFile.string()); + diagnostics.level = Level::no_logs; + } + } else { + problems.emplace_back("Unable to create diagnostics directory " + + diagFile.parent_path().string() + ": " + ec.message()); + diagnostics.level = Level::no_logs; + } + } } for (const auto& problem : problems) { LOG_ERROR("{}", problem); @@ -137,7 +157,7 @@ void Log::flushImpl() { } Log::LoggerConfiguration Log::getLoggerConfigurationImpl(std::string_view name) { - LoggerConfiguration ret{session, std::string{logRootName}}; + LoggerConfiguration ret{session, std::string{programName}}; ret.fullyQualifiedName += '/'; ret.fullyQualifiedName += name; ret.level = std::min({binary.level, text.level, console.level}); @@ -153,6 +173,7 @@ Log& Log::write(const char* buffer, std::streamsize size) { if (text) text.write(buffer, size); if (binary) binary.write(buffer, size); if (console) console.write(buffer, size); + if (diagnostics) diagnostics.write(buffer, size); return *this; } diff --git a/swift/log/SwiftLogging.h b/swift/log/SwiftLogging.h index 41ace7648e42..8ce105096e7a 100644 --- a/swift/log/SwiftLogging.h +++ b/swift/log/SwiftLogging.h @@ -12,6 +12,8 @@ #include #include +#include "swift/log/SwiftDiagnostics.h" + // Logging macros. These will call `logger()` to get a Logger instance, picking up any `logger` // defined in the current scope. Domain-specific loggers can be added or used by either: // * providing a class field called `logger` (as `Logger::operator()()` returns itself) @@ -20,23 +22,39 @@ // * passing a logger around using a `Logger& logger` function parameter // They are created with a name that appears in the logs and can be used to filter debug levels (see // `Logger`). -#define LOG_CRITICAL(...) LOG_WITH_LEVEL(codeql::Log::Level::critical, __VA_ARGS__) -#define LOG_ERROR(...) LOG_WITH_LEVEL(codeql::Log::Level::error, __VA_ARGS__) -#define LOG_WARNING(...) LOG_WITH_LEVEL(codeql::Log::Level::warning, __VA_ARGS__) -#define LOG_INFO(...) LOG_WITH_LEVEL(codeql::Log::Level::info, __VA_ARGS__) -#define LOG_DEBUG(...) LOG_WITH_LEVEL(codeql::Log::Level::debug, __VA_ARGS__) -#define LOG_TRACE(...) LOG_WITH_LEVEL(codeql::Log::Level::trace, __VA_ARGS__) +#define LOG_CRITICAL(...) LOG_WITH_LEVEL(critical, __VA_ARGS__) +#define LOG_ERROR(...) LOG_WITH_LEVEL(error, __VA_ARGS__) +#define LOG_WARNING(...) LOG_WITH_LEVEL(warning, __VA_ARGS__) +#define LOG_INFO(...) LOG_WITH_LEVEL(info, __VA_ARGS__) +#define LOG_DEBUG(...) LOG_WITH_LEVEL(debug, __VA_ARGS__) +#define LOG_TRACE(...) LOG_WITH_LEVEL(trace, __VA_ARGS__) // only do the actual logging if the picked up `Logger` instance is configured to handle the // provided log level. `LEVEL` must be a compile-time constant. `logger()` is evaluated once -#define LOG_WITH_LEVEL(LEVEL, ...) \ - do { \ - constexpr codeql::Log::Level _level = LEVEL; \ - codeql::Logger& _logger = logger(); \ - if (_level >= _logger.level()) { \ - BINLOG_CREATE_SOURCE_AND_EVENT(_logger.writer(), _level, /* category */, binlog::clockNow(), \ - __VA_ARGS__); \ - } \ +#define LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, CATEGORY, ...) \ + do { \ + constexpr codeql::Log::Level _level = codeql::Log::Level::LEVEL; \ + codeql::Logger& _logger = logger(); \ + if (_level >= _logger.level()) { \ + BINLOG_CREATE_SOURCE_AND_EVENT(_logger.writer(), _level, CATEGORY, binlog::clockNow(), \ + __VA_ARGS__); \ + } \ + } while (false) + +#define LOG_WITH_LEVEL(LEVEL, ...) LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, , __VA_ARGS__) + +// Emit errors with a specified diagnostics ID. This must be the name of a function in the +// codeql::diagnostics namespace, which must call SwiftDiagnosticSource::create with ID as first +// argument. This function will be called at most once during the program execution. +// See codeql::diagnostics::internal_error below as an example. +#define DIAGNOSE_CRITICAL(ID, ...) DIAGNOSE_WITH_LEVEL(critical, ID, __VA_ARGS__) +#define DIAGNOSE_ERROR(ID, ...) DIAGNOSE_WITH_LEVEL(error, ID, __VA_ARGS__) + +#define DIAGNOSE_WITH_LEVEL(LEVEL, ID, ...) \ + do { \ + static int _ignore = (codeql::diagnostics::ID(), 0); \ + std::ignore = _ignore; \ + LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, ID, __VA_ARGS__); \ } while (false) // avoid calling into binlog's original macros @@ -68,7 +86,7 @@ namespace codeql { // tools should define this to tweak the root name of all loggers -extern const std::string_view logRootName; +extern const std::string_view programName; // This class is responsible for the global log state (outputs, log level rules, flushing) // State is stored in the singleton `Log::instance()`. @@ -76,7 +94,7 @@ extern const std::string_view logRootName; // `Log::configure("extractor")`). Then, `Log::flush()` should be regularly called. // Logging is configured upon first usage. This consists in // * using environment variable `CODEQL_EXTRACTOR_SWIFT_LOG_DIR` to choose where to dump the log -// file(s). Log files will go to a subdirectory thereof named after `logRootName` +// file(s). Log files will go to a subdirectory thereof named after `programName` // * using environment variable `CODEQL_EXTRACTOR_SWIFT_LOG_LEVELS` to configure levels for // loggers and outputs. This must have the form of a comma separated `spec:level` list, where // `spec` is either a glob pattern (made up of alphanumeric, `/`, `*` and `.` characters) for @@ -122,23 +140,40 @@ class Log { friend binlog::Session; Log& write(const char* buffer, std::streamsize size); + struct OnlyWithCategory {}; + // Output filtered according to a configured log level template struct FilteredOutput { binlog::Severity level; Output output; - binlog::EventFilter filter{ - [this](const binlog::EventSource& src) { return src.severity >= level; }}; + binlog::EventFilter filter; template FilteredOutput(Level level, Args&&... args) - : level{level}, output{std::forward(args)...} {} + : level{level}, output{std::forward(args)...}, filter{filterOnLevel()} {} + + template + FilteredOutput(OnlyWithCategory, Level level, Args&&... args) + : level{level}, + output{std::forward(args)...}, + filter{filterOnLevelAndNonEmptyCategory()} {} FilteredOutput& write(const char* buffer, std::streamsize size) { filter.writeAllowed(buffer, size, output); return *this; } + binlog::EventFilter::Predicate filterOnLevel() const { + return [this](const binlog::EventSource& src) { return src.severity >= level; }; + } + + binlog::EventFilter::Predicate filterOnLevelAndNonEmptyCategory() const { + return [this](const binlog::EventSource& src) { + return !src.category.empty() && src.severity >= level; + }; + } + // if configured as `no_logs`, the output is effectively disabled explicit operator bool() const { return level < Level::no_logs; } }; @@ -151,14 +186,15 @@ class Log { FilteredOutput binary{Level::no_logs}; FilteredOutput text{Level::info, textFile, format}; FilteredOutput console{Level::warning, std::cerr, format}; + FilteredOutput diagnostics{OnlyWithCategory{}, Level::error}; LevelRules sourceRules; - std::vector collectSeverityRulesAndReturnProblems(const char* envVar); + std::vector collectLevelRulesAndReturnProblems(const char* envVar); }; // This class represent a named domain-specific logger, responsible for pushing logs using the // underlying `binlog::SessionWriter` class. This has a configured log level, so that logs on this // `Logger` with a level lower than the configured one are no-ops. The level is configured based -// on rules matching `/` in `CODEQL_EXTRACTOR_SWIFT_LOG_LEVELS` (see above). +// on rules matching `/` in `CODEQL_EXTRACTOR_SWIFT_LOG_LEVELS` (see above). // `` is provided in the constructor. If no rule matches the name, the log level defaults to // the minimum level of all outputs. class Logger { From 8873e42cb1679b3b793d09be99e7d2dc57fc7f9f Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Wed, 3 May 2023 16:02:26 +0200 Subject: [PATCH 04/10] Swift: removed unused `date` dependency --- swift/log/BUILD.bazel | 1 - swift/log/SwiftDiagnostics.cpp | 1 - swift/third_party/load.bzl | 8 -------- 3 files changed, 10 deletions(-) diff --git a/swift/log/BUILD.bazel b/swift/log/BUILD.bazel index a2532ea75f1d..6edc8f795f58 100644 --- a/swift/log/BUILD.bazel +++ b/swift/log/BUILD.bazel @@ -5,7 +5,6 @@ cc_library( visibility = ["//visibility:public"], deps = [ "@binlog", - "@date", "@json", ], ) diff --git a/swift/log/SwiftDiagnostics.cpp b/swift/log/SwiftDiagnostics.cpp index ce29d0c57d61..bf22aee73762 100644 --- a/swift/log/SwiftDiagnostics.cpp +++ b/swift/log/SwiftDiagnostics.cpp @@ -1,6 +1,5 @@ #include "swift/log/SwiftDiagnostics.h" -#include #include #include diff --git a/swift/third_party/load.bzl b/swift/third_party/load.bzl index a85ede792e37..cc5de3427a05 100644 --- a/swift/third_party/load.bzl +++ b/swift/third_party/load.bzl @@ -70,11 +70,3 @@ def load_dependencies(workspace_name): commit = "6af826d0bdb55e4b69e3ad817576745335f243ca", sha256 = "702bb0231a5e21c0374230fed86c8ae3d07ee50f34ffd420e7f8249854b7d85b", ) - - _github_archive( - name = "date", - build_file = _build(workspace_name, "date"), - repository = "HowardHinnant/date", - commit = "6e921e1b1d21e84a5c82416ba7ecd98e33a436d0", - sha256 = "484c450ea1cec479716f7cfce9a54da1867dd4043dde08e7c262b812561fe3bc", - ) From a30d5f5030841974a869427fbcd7df13468aa80f Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Wed, 3 May 2023 16:14:22 +0200 Subject: [PATCH 05/10] Swift: fix diagnostic source creation being called really once --- swift/extractor/main.cpp | 1 - swift/log/SwiftDiagnostics.h | 7 +++++++ swift/log/SwiftLogging.h | 9 ++++----- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/swift/extractor/main.cpp b/swift/extractor/main.cpp index a9159e034ec1..2a4aee016f61 100644 --- a/swift/extractor/main.cpp +++ b/swift/extractor/main.cpp @@ -220,7 +220,6 @@ int main(int argc, char** argv, char** envp) { codeql::Logger logger{"main"}; LOG_INFO("calling extractor with arguments \"{}\"", argDump(argc, argv)); LOG_DEBUG("environment:\n{}\n", envDump(envp)); - DIAGNOSE_ERROR(internal_error, "prout {}", 42); } auto openInterception = codeql::setupFileInterception(configuration); diff --git a/swift/log/SwiftDiagnostics.h b/swift/log/SwiftDiagnostics.h index 5e0953761168..33f9b7896104 100644 --- a/swift/log/SwiftDiagnostics.h +++ b/swift/log/SwiftDiagnostics.h @@ -83,4 +83,11 @@ inline void internal_error() { } } // namespace diagnostics +namespace detail { +template +inline void createSwiftDiagnosticsSourceOnce() { + static int ignore = (Func(), 0); + std::ignore = ignore; +} +} // namespace detail } // namespace codeql diff --git a/swift/log/SwiftLogging.h b/swift/log/SwiftLogging.h index 8ce105096e7a..d41e5f91cfcf 100644 --- a/swift/log/SwiftLogging.h +++ b/swift/log/SwiftLogging.h @@ -50,11 +50,10 @@ #define DIAGNOSE_CRITICAL(ID, ...) DIAGNOSE_WITH_LEVEL(critical, ID, __VA_ARGS__) #define DIAGNOSE_ERROR(ID, ...) DIAGNOSE_WITH_LEVEL(error, ID, __VA_ARGS__) -#define DIAGNOSE_WITH_LEVEL(LEVEL, ID, ...) \ - do { \ - static int _ignore = (codeql::diagnostics::ID(), 0); \ - std::ignore = _ignore; \ - LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, ID, __VA_ARGS__); \ +#define DIAGNOSE_WITH_LEVEL(LEVEL, ID, ...) \ + do { \ + codeql::detail::createSwiftDiagnosticsSourceOnce(); \ + LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, ID, __VA_ARGS__); \ } while (false) // avoid calling into binlog's original macros From bd303357f76f061ab8e28eacd86dcd739d4284f1 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 4 May 2023 10:13:39 +0200 Subject: [PATCH 06/10] Swift: refactor after review --- swift/log/BUILD.bazel | 1 + swift/log/SwiftDiagnostics.cpp | 77 +++++++++++++--------------------- swift/log/SwiftDiagnostics.h | 70 ++++++++++++++----------------- swift/log/SwiftLogging.h | 14 +++---- 4 files changed, 69 insertions(+), 93 deletions(-) diff --git a/swift/log/BUILD.bazel b/swift/log/BUILD.bazel index 6edc8f795f58..7e9b84168da0 100644 --- a/swift/log/BUILD.bazel +++ b/swift/log/BUILD.bazel @@ -4,6 +4,7 @@ cc_library( hdrs = glob(["*.h"]), visibility = ["//visibility:public"], deps = [ + "@absl//absl/strings", "@binlog", "@json", ], diff --git a/swift/log/SwiftDiagnostics.cpp b/swift/log/SwiftDiagnostics.cpp index bf22aee73762..a75cca5d5ae5 100644 --- a/swift/log/SwiftDiagnostics.cpp +++ b/swift/log/SwiftDiagnostics.cpp @@ -2,57 +2,41 @@ #include #include +#include "absl/strings/str_join.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" namespace codeql { -SwiftDiagnosticsSource::SwiftDiagnosticsSource(std::string_view internalId, - std::string&& name, - std::vector&& helpLinks, - std::string&& action) - : name{std::move(name)}, helpLinks{std::move(helpLinks)}, action{std::move(action)} { - id = extractorName; - id += '/'; - id += programName; - id += '/'; - std::transform(internalId.begin(), internalId.end(), std::back_inserter(id), - [](char c) { return c == '_' ? '-' : c; }); -} - -void SwiftDiagnosticsSource::create(std::string_view id, - std::string name, - std::vector helpLinks, - std::string action) { - auto [it, inserted] = map().emplace( - id, SwiftDiagnosticsSource{id, std::move(name), std::move(helpLinks), std::move(action)}); - assert(inserted); -} - void SwiftDiagnosticsSource::emit(std::ostream& out, std::string_view timestamp, std::string_view message) const { - nlohmann::json entry; - auto& source = entry["source"]; - source["id"] = id; - source["name"] = name; - source["extractorName"] = extractorName; - - auto& visibility = entry["visibility"]; - visibility["statusPage"] = true; - visibility["cliSummaryTable"] = true; - visibility["telemetry"] = true; - - entry["severity"] = "error"; - entry["helpLinks"] = helpLinks; - std::string plaintextMessage{message}; - plaintextMessage += ".\n\n"; - plaintextMessage += action; - plaintextMessage += '.'; - entry["plaintextMessage"] = plaintextMessage; - - entry["timestamp"] = timestamp; - + nlohmann::json entry = { + {"source", + { + {"id", sourceId()}, + {"name", name}, + {"extractorName", extractorName}, + }}, + {"visibility", + { + {"statusPage", true}, + {"cliSummaryTable", true}, + {"telemetry", true}, + }}, + {"severity", "error"}, + {"helpLinks", std::vector(absl::StrSplit(helpLinks, ' '))}, + {"plaintextMessage", absl::StrCat(message, ".\n\n", action, ".")}, + {"timestamp", timestamp}, + }; out << entry << '\n'; } +std::string SwiftDiagnosticsSource::sourceId() const { + auto ret = absl::StrJoin({extractorName, programName, id}, "/"); + std::replace(ret.begin(), ret.end(), '_', '-'); + return ret; +} + void SwiftDiagnosticsDumper::write(const char* buffer, std::size_t bufferSize) { binlog::Range range{buffer, bufferSize}; binlog::RangeEntryStream input{range}; @@ -60,12 +44,11 @@ void SwiftDiagnosticsDumper::write(const char* buffer, std::size_t bufferSize) { const auto& source = SwiftDiagnosticsSource::get(event->source->category); std::ostringstream oss; timestampedMessagePrinter.printEvent(oss, *event, events.writerProp(), events.clockSync()); + // TODO(C++20) use oss.view() directly auto data = oss.str(); std::string_view view = data; - auto sep = view.find(' '); - assert(sep != std::string::npos); - auto timestamp = view.substr(0, sep); - auto message = view.substr(sep + 1); + using ViewPair = std::pair; + auto [timestamp, message] = ViewPair(absl::StrSplit(view, absl::MaxSplits(' ', 1))); source.emit(output, timestamp, message); } } diff --git a/swift/log/SwiftDiagnostics.h b/swift/log/SwiftDiagnostics.h index 33f9b7896104..73e81af26bfe 100644 --- a/swift/log/SwiftDiagnostics.h +++ b/swift/log/SwiftDiagnostics.h @@ -8,6 +8,8 @@ #include #include #include +#include +#include namespace codeql { @@ -16,16 +18,27 @@ extern const std::string_view programName; // Models a diagnostic source for Swift, holding static information that goes out into a diagnostic // These are internally stored into a map on id's. A specific error log can use binlog's category // as id, which will then be used to recover the diagnostic source while dumping. -class SwiftDiagnosticsSource { - public: - // creates a SwiftDiagnosticsSource with the given data - static void create(std::string_view id, - std::string name, - std::vector helpLinks, - std::string action); +struct SwiftDiagnosticsSource { + std::string_view id; + std::string_view name; + static constexpr std::string_view extractorName = "swift"; + std::string_view action; + std::string_view helpLinks; // space separated if more than 1. Not a vector to allow constexpr + + // for the moment, we only output errors, so no need to store the severity - // gets a previously created SwiftDiagnosticsSource for the given id. Will abort if none exists - static const SwiftDiagnosticsSource& get(const std::string& id) { return map().at(id); } + // registers a diagnostics source for later retrieval with get, if not done yet + template + static void inscribe() { + static std::once_flag once; + std::call_once(once, [] { + auto [it, inserted] = map().emplace(Spec->id, Spec); + assert(inserted); + }); + } + + // gets a previously inscribed SwiftDiagnosticsSource for the given id. Will abort if none exists + static const SwiftDiagnosticsSource& get(const std::string& id) { return *map().at(id); } // emit a JSON diagnostics for this source with the given timestamp and message to out // A plaintextMessage is used that includes both the message and the action to take. Dots are @@ -34,26 +47,13 @@ class SwiftDiagnosticsSource { void emit(std::ostream& out, std::string_view timestamp, std::string_view message) const; private: - using Map = std::unordered_map; - - std::string id; - std::string name; - static constexpr std::string_view extractorName = "swift"; - - // for the moment, we only output errors, so no need to store the severity - - std::vector helpLinks; - std::string action; + std::string sourceId() const; + using Map = std::unordered_map; static Map& map() { static Map ret; return ret; } - - SwiftDiagnosticsSource(std::string_view internalId, - std::string&& name, - std::vector&& helpLinks, - std::string&& action); }; // An output modeling binlog's output stream concept that intercepts binlog entries and translates @@ -76,18 +76,12 @@ class SwiftDiagnosticsDumper { binlog::PrettyPrinter timestampedMessagePrinter{"%u %m", "%Y-%m-%dT%H:%M:%S.%NZ"}; }; -namespace diagnostics { -inline void internal_error() { - SwiftDiagnosticsSource::create("internal_error", "Internal error", {}, - "Contact us about this issue"); -} -} // namespace diagnostics - -namespace detail { -template -inline void createSwiftDiagnosticsSourceOnce() { - static int ignore = (Func(), 0); - std::ignore = ignore; -} -} // namespace detail } // namespace codeql + +namespace codeql_diagnostics { +constexpr codeql::SwiftDiagnosticsSource internal_error{ + "internal_error", + "Internal error", + "Contact us about this issue", +}; +} // namespace codeql_diagnostics diff --git a/swift/log/SwiftLogging.h b/swift/log/SwiftLogging.h index d41e5f91cfcf..bcd18086b3af 100644 --- a/swift/log/SwiftLogging.h +++ b/swift/log/SwiftLogging.h @@ -43,17 +43,15 @@ #define LOG_WITH_LEVEL(LEVEL, ...) LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, , __VA_ARGS__) -// Emit errors with a specified diagnostics ID. This must be the name of a function in the -// codeql::diagnostics namespace, which must call SwiftDiagnosticSource::create with ID as first -// argument. This function will be called at most once during the program execution. -// See codeql::diagnostics::internal_error below as an example. +// Emit errors with a specified diagnostics ID. This must be the name of a `SwiftDiagnosticsSource` +// defined in the `codeql_diagnostics` namespace, which must have `id` equal to its name. #define DIAGNOSE_CRITICAL(ID, ...) DIAGNOSE_WITH_LEVEL(critical, ID, __VA_ARGS__) #define DIAGNOSE_ERROR(ID, ...) DIAGNOSE_WITH_LEVEL(error, ID, __VA_ARGS__) -#define DIAGNOSE_WITH_LEVEL(LEVEL, ID, ...) \ - do { \ - codeql::detail::createSwiftDiagnosticsSourceOnce(); \ - LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, ID, __VA_ARGS__); \ +#define DIAGNOSE_WITH_LEVEL(LEVEL, ID, ...) \ + do { \ + codeql::SwiftDiagnosticsSource::inscribe<&codeql_diagnostics::ID>(); \ + LOG_WITH_LEVEL_AND_CATEGORY(LEVEL, ID, __VA_ARGS__); \ } while (false) // avoid calling into binlog's original macros From bce483ddb1d250eebf8b0881c29ae1c859554004 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 4 May 2023 10:39:50 +0200 Subject: [PATCH 07/10] Swift: rename `log` package to `logging` --- swift/extractor/SwiftExtractor.cpp | 2 +- swift/extractor/infra/BUILD.bazel | 2 +- swift/extractor/infra/SwiftDispatcher.h | 2 +- swift/extractor/infra/file/BUILD.bazel | 2 +- swift/extractor/infra/file/FsLogger.h | 2 +- swift/extractor/infra/file/TargetFile.cpp | 4 ++-- swift/extractor/invocation/SwiftInvocationExtractor.cpp | 2 +- swift/extractor/main.cpp | 2 +- swift/extractor/remapping/SwiftFileInterception.cpp | 2 +- swift/extractor/trap/BUILD.bazel | 2 +- swift/extractor/trap/TrapDomain.h | 2 +- swift/{log => logging}/BUILD.bazel | 2 +- swift/{log => logging}/SwiftAssert.h | 2 +- swift/{log => logging}/SwiftDiagnostics.cpp | 2 +- swift/{log => logging}/SwiftDiagnostics.h | 0 swift/{log => logging}/SwiftLogging.cpp | 2 +- swift/{log => logging}/SwiftLogging.h | 2 +- 17 files changed, 17 insertions(+), 17 deletions(-) rename swift/{log => logging}/BUILD.bazel (89%) rename swift/{log => logging}/SwiftAssert.h (97%) rename swift/{log => logging}/SwiftDiagnostics.cpp (97%) rename swift/{log => logging}/SwiftDiagnostics.h (100%) rename swift/{log => logging}/SwiftLogging.cpp (99%) rename swift/{log => logging}/SwiftLogging.h (99%) diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index 34772074df13..77e27c00f418 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -15,7 +15,7 @@ #include "swift/extractor/infra/SwiftLocationExtractor.h" #include "swift/extractor/infra/SwiftBodyEmissionStrategy.h" #include "swift/extractor/mangler/SwiftMangler.h" -#include "swift/log/SwiftAssert.h" +#include "swift/logging/SwiftAssert.h" using namespace codeql; using namespace std::string_literals; diff --git a/swift/extractor/infra/BUILD.bazel b/swift/extractor/infra/BUILD.bazel index 1d1b94a759cd..f2b7f6baee2c 100644 --- a/swift/extractor/infra/BUILD.bazel +++ b/swift/extractor/infra/BUILD.bazel @@ -9,7 +9,7 @@ swift_cc_library( "//swift/extractor/config", "//swift/extractor/infra/file", "//swift/extractor/trap", - "//swift/log", + "//swift/logging", "//swift/third_party/swift-llvm-support", ], ) diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h index b3bbb1e7b4ab..33b0614e4353 100644 --- a/swift/extractor/infra/SwiftDispatcher.h +++ b/swift/extractor/infra/SwiftDispatcher.h @@ -13,7 +13,7 @@ #include "swift/extractor/infra/SwiftBodyEmissionStrategy.h" #include "swift/extractor/infra/SwiftMangledName.h" #include "swift/extractor/config/SwiftExtractorState.h" -#include "swift/log/SwiftAssert.h" +#include "swift/logging/SwiftAssert.h" namespace codeql { diff --git a/swift/extractor/infra/file/BUILD.bazel b/swift/extractor/infra/file/BUILD.bazel index 4c29f9afebe0..65cfea339950 100644 --- a/swift/extractor/infra/file/BUILD.bazel +++ b/swift/extractor/infra/file/BUILD.bazel @@ -11,7 +11,7 @@ swift_cc_library( exclude = ["FsLogger.h"], ) + [":path_hash_workaround"], visibility = ["//swift:__subpackages__"], - deps = ["//swift/log"], + deps = ["//swift/logging"], ) genrule( diff --git a/swift/extractor/infra/file/FsLogger.h b/swift/extractor/infra/file/FsLogger.h index bfb245d97279..5797b8929821 100644 --- a/swift/extractor/infra/file/FsLogger.h +++ b/swift/extractor/infra/file/FsLogger.h @@ -1,4 +1,4 @@ -#include "swift/log/SwiftLogging.h" +#include "swift/logging/SwiftLogging.h" namespace codeql { namespace fs_logger { diff --git a/swift/extractor/infra/file/TargetFile.cpp b/swift/extractor/infra/file/TargetFile.cpp index 1261dc1a4930..4ce4c8ae23be 100644 --- a/swift/extractor/infra/file/TargetFile.cpp +++ b/swift/extractor/infra/file/TargetFile.cpp @@ -1,7 +1,7 @@ #include "swift/extractor/infra/file/TargetFile.h" #include "swift/extractor/infra/file/FsLogger.h" -#include "swift/log/SwiftLogging.h" -#include "swift/log/SwiftAssert.h" +#include "swift/logging/SwiftLogging.h" +#include "swift/logging/SwiftAssert.h" #include #include diff --git a/swift/extractor/invocation/SwiftInvocationExtractor.cpp b/swift/extractor/invocation/SwiftInvocationExtractor.cpp index c2d77d912899..3ec5b9bf0a4e 100644 --- a/swift/extractor/invocation/SwiftInvocationExtractor.cpp +++ b/swift/extractor/invocation/SwiftInvocationExtractor.cpp @@ -4,7 +4,7 @@ #include "swift/extractor/trap/generated/TrapTags.h" #include "swift/extractor/infra/file/TargetFile.h" #include "swift/extractor/infra/file/Path.h" -#include "swift/log/SwiftAssert.h" +#include "swift/logging/SwiftAssert.h" #include "swift/extractor/mangler/SwiftMangler.h" namespace fs = std::filesystem; diff --git a/swift/extractor/main.cpp b/swift/extractor/main.cpp index 2a040db5bb7b..f195193e5d7e 100644 --- a/swift/extractor/main.cpp +++ b/swift/extractor/main.cpp @@ -18,7 +18,7 @@ #include "swift/extractor/invocation/SwiftInvocationExtractor.h" #include "swift/extractor/trap/TrapDomain.h" #include "swift/extractor/infra/file/Path.h" -#include "swift/log/SwiftAssert.h" +#include "swift/logging/SwiftAssert.h" using namespace std::string_literals; using namespace codeql::main_logger; diff --git a/swift/extractor/remapping/SwiftFileInterception.cpp b/swift/extractor/remapping/SwiftFileInterception.cpp index fed3b297e150..7b22aabec8f3 100644 --- a/swift/extractor/remapping/SwiftFileInterception.cpp +++ b/swift/extractor/remapping/SwiftFileInterception.cpp @@ -14,7 +14,7 @@ #include "swift/extractor/infra/file/PathHash.h" #include "swift/extractor/infra/file/Path.h" -#include "swift/log/SwiftAssert.h" +#include "swift/logging/SwiftAssert.h" #ifdef __APPLE__ // path is hardcoded as otherwise redirection could break when setting DYLD_FALLBACK_LIBRARY_PATH diff --git a/swift/extractor/trap/BUILD.bazel b/swift/extractor/trap/BUILD.bazel index 8ac63aba0855..f156aa9c9845 100644 --- a/swift/extractor/trap/BUILD.bazel +++ b/swift/extractor/trap/BUILD.bazel @@ -52,7 +52,7 @@ swift_cc_library( visibility = ["//visibility:public"], deps = [ "//swift/extractor/infra/file", - "//swift/log", + "//swift/logging", "@absl//absl/numeric:bits", ], ) diff --git a/swift/extractor/trap/TrapDomain.h b/swift/extractor/trap/TrapDomain.h index d1c3deca431d..2ca5efa113de 100644 --- a/swift/extractor/trap/TrapDomain.h +++ b/swift/extractor/trap/TrapDomain.h @@ -5,7 +5,7 @@ #include "swift/extractor/trap/TrapLabel.h" #include "swift/extractor/infra/file/TargetFile.h" -#include "swift/log/SwiftLogging.h" +#include "swift/logging/SwiftLogging.h" #include "swift/extractor/infra/SwiftMangledName.h" namespace codeql { diff --git a/swift/log/BUILD.bazel b/swift/logging/BUILD.bazel similarity index 89% rename from swift/log/BUILD.bazel rename to swift/logging/BUILD.bazel index 7e9b84168da0..598d3a3aa317 100644 --- a/swift/log/BUILD.bazel +++ b/swift/logging/BUILD.bazel @@ -1,5 +1,5 @@ cc_library( - name = "log", + name = "logging", srcs = glob(["*.cpp"]), hdrs = glob(["*.h"]), visibility = ["//visibility:public"], diff --git a/swift/log/SwiftAssert.h b/swift/logging/SwiftAssert.h similarity index 97% rename from swift/log/SwiftAssert.h rename to swift/logging/SwiftAssert.h index fa632e3fd4b8..9ffb3c5f860d 100644 --- a/swift/log/SwiftAssert.h +++ b/swift/logging/SwiftAssert.h @@ -2,7 +2,7 @@ #include -#include "swift/log/SwiftLogging.h" +#include "swift/logging/SwiftLogging.h" // assert CONDITION, which is always evaluated (once) regardless of the build type. If // CONDITION is not satisfied, emit a critical log optionally using provided format and arguments, diff --git a/swift/log/SwiftDiagnostics.cpp b/swift/logging/SwiftDiagnostics.cpp similarity index 97% rename from swift/log/SwiftDiagnostics.cpp rename to swift/logging/SwiftDiagnostics.cpp index a75cca5d5ae5..0c3ae0fe88e2 100644 --- a/swift/log/SwiftDiagnostics.cpp +++ b/swift/logging/SwiftDiagnostics.cpp @@ -1,4 +1,4 @@ -#include "swift/log/SwiftDiagnostics.h" +#include "swift/logging/SwiftDiagnostics.h" #include #include diff --git a/swift/log/SwiftDiagnostics.h b/swift/logging/SwiftDiagnostics.h similarity index 100% rename from swift/log/SwiftDiagnostics.h rename to swift/logging/SwiftDiagnostics.h diff --git a/swift/log/SwiftLogging.cpp b/swift/logging/SwiftLogging.cpp similarity index 99% rename from swift/log/SwiftLogging.cpp rename to swift/logging/SwiftLogging.cpp index 9dff83ec2458..4f3e3607f2b9 100644 --- a/swift/log/SwiftLogging.cpp +++ b/swift/logging/SwiftLogging.cpp @@ -1,4 +1,4 @@ -#include "swift/log/SwiftLogging.h" +#include "swift/logging/SwiftLogging.h" #include #include diff --git a/swift/log/SwiftLogging.h b/swift/logging/SwiftLogging.h similarity index 99% rename from swift/log/SwiftLogging.h rename to swift/logging/SwiftLogging.h index 24114b1b6910..cf756c9e5a0c 100644 --- a/swift/log/SwiftLogging.h +++ b/swift/logging/SwiftLogging.h @@ -13,7 +13,7 @@ #include #include -#include "swift/log/SwiftDiagnostics.h" +#include "swift/logging/SwiftDiagnostics.h" // Logging macros. These will call `logger()` to get a Logger instance, picking up any `logger` // defined in the current scope. Domain-specific loggers can be added or used by either: From d61e3664416170eef1796c21e0e0c25a3dbe4f59 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 4 May 2023 12:15:58 +0200 Subject: [PATCH 08/10] Swift: replace `assert` with `CODEQL_ASSERT` --- swift/logging/SwiftDiagnostics.cpp | 13 +++++++++++++ swift/logging/SwiftDiagnostics.h | 7 +++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/swift/logging/SwiftDiagnostics.cpp b/swift/logging/SwiftDiagnostics.cpp index 0c3ae0fe88e2..9a20830e1ff6 100644 --- a/swift/logging/SwiftDiagnostics.cpp +++ b/swift/logging/SwiftDiagnostics.cpp @@ -5,8 +5,17 @@ #include "absl/strings/str_join.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" +#include "swift/logging/SwiftAssert.h" namespace codeql { + +namespace { +Logger& logger() { + static Logger ret{"diagnostics"}; + return ret; +} +} // namespace + void SwiftDiagnosticsSource::emit(std::ostream& out, std::string_view timestamp, std::string_view message) const { @@ -36,6 +45,10 @@ std::string SwiftDiagnosticsSource::sourceId() const { std::replace(ret.begin(), ret.end(), '_', '-'); return ret; } +void SwiftDiagnosticsSource::inscribeImpl(const SwiftDiagnosticsSource* source) { + auto [it, inserted] = map().emplace(source->id, source); + CODEQL_ASSERT(inserted, "duplicate diagnostics source detected with id {}", source->id); +} void SwiftDiagnosticsDumper::write(const char* buffer, std::size_t bufferSize) { binlog::Range range{buffer, bufferSize}; diff --git a/swift/logging/SwiftDiagnostics.h b/swift/logging/SwiftDiagnostics.h index 73e81af26bfe..566b56ad99a4 100644 --- a/swift/logging/SwiftDiagnostics.h +++ b/swift/logging/SwiftDiagnostics.h @@ -31,10 +31,7 @@ struct SwiftDiagnosticsSource { template static void inscribe() { static std::once_flag once; - std::call_once(once, [] { - auto [it, inserted] = map().emplace(Spec->id, Spec); - assert(inserted); - }); + std::call_once(once, inscribeImpl, Spec); } // gets a previously inscribed SwiftDiagnosticsSource for the given id. Will abort if none exists @@ -47,6 +44,8 @@ struct SwiftDiagnosticsSource { void emit(std::ostream& out, std::string_view timestamp, std::string_view message) const; private: + static void inscribeImpl(const SwiftDiagnosticsSource* Spec); + std::string sourceId() const; using Map = std::unordered_map; From b5c0cd8cacb986e79a2440a3da7d5085f45784ea Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 4 May 2023 12:18:02 +0200 Subject: [PATCH 09/10] Swift: remove unused third party build file --- swift/third_party/BUILD.date.bazel | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 swift/third_party/BUILD.date.bazel diff --git a/swift/third_party/BUILD.date.bazel b/swift/third_party/BUILD.date.bazel deleted file mode 100644 index 598d9efea3b2..000000000000 --- a/swift/third_party/BUILD.date.bazel +++ /dev/null @@ -1,6 +0,0 @@ -cc_library( - name = "date", - hdrs = glob(["include/date/*.h"]), - includes = ["include"], - visibility = ["//visibility:public"], -) From 7ce1189e36961c8e554242a1b8ffa5f0a398bbcb Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 4 May 2023 15:14:46 +0200 Subject: [PATCH 10/10] Swift: tweak after review comments --- swift/logging/SwiftDiagnostics.cpp | 2 +- swift/logging/SwiftDiagnostics.h | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/swift/logging/SwiftDiagnostics.cpp b/swift/logging/SwiftDiagnostics.cpp index 9a20830e1ff6..dfe86f8186f4 100644 --- a/swift/logging/SwiftDiagnostics.cpp +++ b/swift/logging/SwiftDiagnostics.cpp @@ -45,7 +45,7 @@ std::string SwiftDiagnosticsSource::sourceId() const { std::replace(ret.begin(), ret.end(), '_', '-'); return ret; } -void SwiftDiagnosticsSource::inscribeImpl(const SwiftDiagnosticsSource* source) { +void SwiftDiagnosticsSource::registerImpl(const SwiftDiagnosticsSource* source) { auto [it, inserted] = map().emplace(source->id, source); CODEQL_ASSERT(inserted, "duplicate diagnostics source detected with id {}", source->id); } diff --git a/swift/logging/SwiftDiagnostics.h b/swift/logging/SwiftDiagnostics.h index 566b56ad99a4..95c6cb85e5a8 100644 --- a/swift/logging/SwiftDiagnostics.h +++ b/swift/logging/SwiftDiagnostics.h @@ -23,15 +23,17 @@ struct SwiftDiagnosticsSource { std::string_view name; static constexpr std::string_view extractorName = "swift"; std::string_view action; - std::string_view helpLinks; // space separated if more than 1. Not a vector to allow constexpr + // space separated if more than 1. Not a vector to allow constexpr + // TODO(C++20) with vector going constexpr this can be turned to `std::vector` + std::string_view helpLinks; // for the moment, we only output errors, so no need to store the severity // registers a diagnostics source for later retrieval with get, if not done yet - template - static void inscribe() { + template + static void ensureRegistered() { static std::once_flag once; - std::call_once(once, inscribeImpl, Spec); + std::call_once(once, registerImpl, Source); } // gets a previously inscribed SwiftDiagnosticsSource for the given id. Will abort if none exists @@ -44,7 +46,7 @@ struct SwiftDiagnosticsSource { void emit(std::ostream& out, std::string_view timestamp, std::string_view message) const; private: - static void inscribeImpl(const SwiftDiagnosticsSource* Spec); + static void registerImpl(const SwiftDiagnosticsSource* source); std::string sourceId() const; using Map = std::unordered_map;