From b4b372cbc4d1e595adedb461c54189ce85b9cabd Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Fri, 6 Dec 2024 16:48:33 -0800 Subject: [PATCH] Introduce DiagnosticContext, an 'error document' like type. (#1323) * In several PRs, such as https://github.com/microsoft/vcpkg-tool/pull/908 and https://github.com/microsoft/vcpkg-tool/pull/1210 , it has become clear that we need a thread safe channel through which errors and warnings can both be reasonably reported. Now that https://github.com/microsoft/vcpkg-tool/pull/1279 is landed and functionally everything in the codebase uses ExpectedL, we can look at what the new thing that fixes issues is. Consider the following: ```c++ ExpectedL example_api(int a); ExpectedL> try_load_port_manifest_text(StringView text, StringView control_path, MessageSink& warning_sink); ``` The reason this can't return the warnings through the ExpectedL channel is that we don't want the 'error' state to be engaged when there are merely warnings. Moreover, that these channels are different channels means that situations that might want to return errors and warnings together, as happens when parsing files, means that order relationships between errors and warnings is lost. It is probably a good idea in general to put warnings and errors about the same location next to each other in the output, but that's hard to do with this interface. Rather than multiplexing everything through the return value, this proposal is to multiplex only the success or failure through the return value, and report any specific error information through an out parameter. 1. Distinguish whether an overall operation succeeded or failed in the return value, but 2. record any errors or warnings via an out parameter. Applying this to the above gives: ```c++ Optional example_api(MessageContext& context, int a); // unique_ptr is already 'optional' std::unique_ptr try_load_port_manifest_text(MessageContext& context, StringView text, StringView control_path); ``` Issues this new mechanism fixes: * Errors and warnings can share the same channel and thus be printed together * The interface between code wanting to report events and the code wanting to consume them is a natural thread synchronization boundary. Other attempts to fix this have been incorrect by synchronizing individual print calls ( https://github.com/microsoft/vcpkg-tool/pull/1290 ) or complex enough that we are not sure they are correct by trying to recover boundaries by reparsing our own error output ( https://github.com/microsoft/vcpkg-tool/pull/908 ) * This shuts down the "error: error:" and similar bugs where it isn't clear who is formatting the overall error message vs. talking about individual components Known issues that are not fixed by this change: * This still doesn't make it easy for callers to programmatically handle specific types of errors. Currently, we have some APIs that still use explicit `std::error_code` because they want to do different things for 'file does not exist' vs. 'there was an I/O error'. Given that this condition isn't well served by the ExpectedL mechanism I don't want to wait until we have a better solution to it to proceed. * Because we aren't making the context parameter the 'success carrier' it's more complex to implement 'warnings as errors' or similar functionality where the caller decides how 'important' something is. I would be in favor of moving all success tests to the context parameter but I'm not proposing that because the other vcpkg maintainers do not like it. * Contextual information / stack problems aren't solved. However, the context parameter might be extended in the future to help with this. --- include/vcpkg/base/diagnostics.h | 273 ++++++++++++++++++++ include/vcpkg/base/parse.h | 5 +- include/vcpkg/paragraphparser.h | 2 +- include/vcpkg/textrowcol.h | 17 -- src/vcpkg-test/manifests.cpp | 13 +- src/vcpkg-test/messages.cpp | 385 ++++++++++++++++++++++++++++ src/vcpkg-test/util.cpp | 2 +- src/vcpkg/base/diagnostics.cpp | 198 ++++++++++++++ src/vcpkg/base/git.cpp | 2 +- src/vcpkg/base/json.cpp | 7 +- src/vcpkg/base/parse.cpp | 45 +++- src/vcpkg/base/system.mac.cpp | 2 +- src/vcpkg/base/system.process.cpp | 2 +- src/vcpkg/binarycaching.cpp | 4 +- src/vcpkg/cgroup-parser.cpp | 2 +- src/vcpkg/ci-baseline.cpp | 2 +- src/vcpkg/commands.package-info.cpp | 2 +- src/vcpkg/packagespec.cpp | 2 +- src/vcpkg/paragraphs.cpp | 2 +- src/vcpkg/platform-expression.cpp | 2 +- src/vcpkg/sourceparagraph.cpp | 2 +- 21 files changed, 922 insertions(+), 49 deletions(-) create mode 100644 include/vcpkg/base/diagnostics.h delete mode 100644 include/vcpkg/textrowcol.h create mode 100644 src/vcpkg/base/diagnostics.cpp diff --git a/include/vcpkg/base/diagnostics.h b/include/vcpkg/base/diagnostics.h new file mode 100644 index 0000000000..d7d676f5fe --- /dev/null +++ b/include/vcpkg/base/diagnostics.h @@ -0,0 +1,273 @@ +#pragma once + +#include +#include +#include + +#include +#include +#include +#include + +namespace vcpkg +{ + enum class DiagKind + { + None, // foo.h: localized + Message, // foo.h: message: localized + Error, // foo.h: error: localized + Warning, // foo.h: warning: localized + Note, // foo.h: note: localized + COUNT + }; + + struct TextRowCol + { + // '0' indicates that line and column information is unknown; '1' is the first row/column + int row = 0; + int column = 0; + }; + + struct DiagnosticLine + { + template, int> = 0> + DiagnosticLine(DiagKind kind, MessageLike&& message) + : m_kind(kind), m_origin(), m_position(), m_message(std::forward(message)) + { + } + + template, int> = 0> + DiagnosticLine(DiagKind kind, StringView origin, MessageLike&& message) + : m_kind(kind), m_origin(origin.to_string()), m_position(), m_message(std::forward(message)) + { + if (origin.empty()) + { + Checks::unreachable(VCPKG_LINE_INFO, "origin must not be empty"); + } + } + + template, int> = 0> + DiagnosticLine(DiagKind kind, StringView origin, TextRowCol position, MessageLike&& message) + : m_kind(kind) + , m_origin(origin.to_string()) + , m_position(position) + , m_message(std::forward(message)) + { + if (origin.empty()) + { + Checks::unreachable(VCPKG_LINE_INFO, "origin must not be empty"); + } + } + + // Prints this diagnostic to the supplied sink. + void print_to(MessageSink& sink) const; + // Converts this message into a string + // Prefer print() if possible because it applies color + std::string to_string() const; + void to_string(std::string& target) const; + + LocalizedString to_json_reader_string(const std::string& path, const LocalizedString& type) const; + + DiagKind kind() const noexcept { return m_kind; } + + private: + DiagKind m_kind; + Optional m_origin; + TextRowCol m_position; + LocalizedString m_message; + }; + + struct DiagnosticContext + { + virtual void report(const DiagnosticLine& line) = 0; + virtual void report(DiagnosticLine&& line) { report(line); } + + void report_error(const LocalizedString& message) { report(DiagnosticLine{DiagKind::Error, message}); } + void report_error(LocalizedString&& message) { report(DiagnosticLine{DiagKind::Error, std::move(message)}); } + template + void report_error(VCPKG_DECL_MSG_ARGS) + { + LocalizedString message; + msg::format_to(message, VCPKG_EXPAND_MSG_ARGS); + this->report_error(std::move(message)); + } + + protected: + ~DiagnosticContext() = default; + }; + + struct BufferedDiagnosticContext final : DiagnosticContext + { + virtual void report(const DiagnosticLine& line) override; + virtual void report(DiagnosticLine&& line) override; + + std::vector lines; + + // Prints all diagnostics to the supplied sink. + void print_to(MessageSink& sink) const; + // Converts this message into a string + // Prefer print() if possible because it applies color + std::string to_string() const; + void to_string(std::string& target) const; + + bool any_errors() const noexcept; + }; + + extern DiagnosticContext& console_diagnostic_context; + extern DiagnosticContext& null_diagnostic_context; + + // The following overloads are implementing + // adapt_context_to_expected(Fn functor, Args&&... args) + // + // Given: + // Optional functor(DiagnosticContext&, args...), adapts functor to return ExpectedL + // Optional& functor(DiagnosticContext&, args...), adapts functor to return ExpectedL + // Optional&& functor(DiagnosticContext&, args...), adapts functor to return ExpectedL + // std::unique_ptr functor(DiagnosticContext&, args...), adapts functor to return ExpectedL> + + // If Ty is an Optional, typename AdaptContextUnwrapOptional::type is the type necessary to return U, and fwd + // is the type necessary to forward U. Otherwise, there are no members ::type or ::fwd + template + struct AdaptContextUnwrapOptional + { + // no member ::type, SFINAEs out when the input type is: + // * not Optional + // * volatile + }; + + template + struct AdaptContextUnwrapOptional> + { + // prvalue, move from the optional into the expected + using type = Wrapped; + using fwd = Wrapped&&; + }; + + template + struct AdaptContextUnwrapOptional> + { + // const prvalue + using type = Wrapped; + using fwd = const Wrapped&&; + }; + + template + struct AdaptContextUnwrapOptional&> + { + // lvalue, return an expected referencing the Wrapped inside the optional + using type = Wrapped&; + using fwd = Wrapped&; + }; + + template + struct AdaptContextUnwrapOptional&> + { + // const lvalue + using type = const Wrapped&; + using fwd = const Wrapped&; + }; + + template + struct AdaptContextUnwrapOptional&&> + { + // xvalue, move from the optional into the expected + using type = Wrapped; + using fwd = Wrapped&&; + }; + + template + struct AdaptContextUnwrapOptional&&> + { + // const xvalue + using type = Wrapped; + using fwd = const Wrapped&&; + }; + + // The overload for functors that return Optional + template + auto adapt_context_to_expected(Fn functor, Args&&... args) + -> ExpectedL< + typename AdaptContextUnwrapOptional>::type> + { + using Unwrapper = AdaptContextUnwrapOptional>; + using ReturnType = ExpectedL; + BufferedDiagnosticContext bdc; + decltype(auto) maybe_result = functor(bdc, std::forward(args)...); + if (auto result = maybe_result.get()) + { + // N.B.: This may be a move + return ReturnType{static_cast(*result), expected_left_tag}; + } + + return ReturnType{LocalizedString::from_raw(bdc.to_string()), expected_right_tag}; + } + + // If Ty is a std::unique_ptr, typename AdaptContextDetectUniquePtr::type is the type necessary to return + template + struct AdaptContextDetectUniquePtr + { + // no member ::type, SFINAEs out when the input type is: + // * not unique_ptr + // * volatile + }; + + template + struct AdaptContextDetectUniquePtr> + { + // prvalue, move into the Expected + using type = std::unique_ptr; + }; + + template + struct AdaptContextDetectUniquePtr> + { + // const prvalue (not valid, can't be moved from) + // no members + }; + + template + struct AdaptContextDetectUniquePtr&> + { + // lvalue, reference the unique_ptr itself + using type = std::unique_ptr&; + }; + + template + struct AdaptContextDetectUniquePtr&> + { + // const lvalue + using type = const std::unique_ptr&; + }; + + template + struct AdaptContextDetectUniquePtr&&> + { + // xvalue, move into the Expected + using type = std::unique_ptr; + }; + + template + struct AdaptContextDetectUniquePtr&&> + { + // const xvalue (not valid, can't be moved from) + // no members + }; + + // The overload for functors that return std::unique_ptr + template + auto adapt_context_to_expected(Fn functor, Args&&... args) + -> ExpectedL< + typename AdaptContextDetectUniquePtr>::type> + { + using ReturnType = ExpectedL< + typename AdaptContextDetectUniquePtr>::type>; + BufferedDiagnosticContext bdc; + decltype(auto) maybe_result = functor(bdc, std::forward(args)...); + if (maybe_result) + { + return ReturnType{static_cast(maybe_result), expected_left_tag}; + } + + return ReturnType{LocalizedString::from_raw(bdc.to_string()), expected_right_tag}; + } +} diff --git a/include/vcpkg/base/parse.h b/include/vcpkg/base/parse.h index bf01590697..0e7e4c3106 100644 --- a/include/vcpkg/base/parse.h +++ b/include/vcpkg/base/parse.h @@ -2,13 +2,12 @@ #include +#include #include #include #include #include -#include - #include namespace vcpkg @@ -44,7 +43,7 @@ namespace vcpkg struct ParserBase { - ParserBase(StringView text, Optional origin, TextRowCol init_rowcol = {}); + ParserBase(StringView text, Optional origin, TextRowCol init_rowcol); static constexpr bool is_whitespace(char32_t ch) { return ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n'; } static constexpr bool is_lower_alpha(char32_t ch) { return ch >= 'a' && ch <= 'z'; } diff --git a/include/vcpkg/paragraphparser.h b/include/vcpkg/paragraphparser.h index e2a395c13d..97fefb5e1e 100644 --- a/include/vcpkg/paragraphparser.h +++ b/include/vcpkg/paragraphparser.h @@ -2,13 +2,13 @@ #include +#include #include #include #include #include #include -#include #include #include diff --git a/include/vcpkg/textrowcol.h b/include/vcpkg/textrowcol.h deleted file mode 100644 index d7e216d852..0000000000 --- a/include/vcpkg/textrowcol.h +++ /dev/null @@ -1,17 +0,0 @@ -#pragma once - -namespace vcpkg -{ - struct TextRowCol - { - constexpr TextRowCol() noexcept = default; - constexpr TextRowCol(int row, int column) noexcept : row(row), column(column) { } - /// '0' indicates uninitialized; '1' is the first row. - int row = 0; - /// '0' indicates uninitialized; '1' is the first column. - int column = 0; - - constexpr int row_or(int def) const noexcept { return row ? row : def; } - constexpr int column_or(int def) const noexcept { return column ? column : def; } - }; -} diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp index bbb20e0712..9bb732a2b7 100644 --- a/src/vcpkg-test/manifests.cpp +++ b/src/vcpkg-test/manifests.cpp @@ -1326,14 +1326,14 @@ TEST_CASE ("license error messages", "[manifests][license]") ParseMessages messages; parse_spdx_license_expression("", messages); CHECK(messages.error.value_or_exit(VCPKG_LINE_INFO) == - LocalizedString::from_raw(R"(:1:1: error: SPDX license expression was empty. + LocalizedString::from_raw(R"(: error: SPDX license expression was empty. on expression: ^)")); parse_spdx_license_expression("MIT ()", messages); CHECK(messages.error.value_or_exit(VCPKG_LINE_INFO) == LocalizedString::from_raw( - R"(:1:5: error: Expected a compound or the end of the string, found a parenthesis. + R"(: error: Expected a compound or the end of the string, found a parenthesis. on expression: MIT () ^)")); @@ -1341,14 +1341,13 @@ TEST_CASE ("license error messages", "[manifests][license]") CHECK( messages.error.value_or_exit(VCPKG_LINE_INFO) == LocalizedString::from_raw( - R"(:1:5: error: SPDX license expression contains an extra '+'. These are only allowed directly after a license identifier. + R"(: error: SPDX license expression contains an extra '+'. These are only allowed directly after a license identifier. on expression: MIT + ^)")); parse_spdx_license_expression("MIT AND", messages); - CHECK( - messages.error.value_or_exit(VCPKG_LINE_INFO) == - LocalizedString::from_raw(R"(:1:8: error: Expected a license name, found the end of the string. + CHECK(messages.error.value_or_exit(VCPKG_LINE_INFO) == + LocalizedString::from_raw(R"(: error: Expected a license name, found the end of the string. on expression: MIT AND ^)")); @@ -1357,7 +1356,7 @@ TEST_CASE ("license error messages", "[manifests][license]") REQUIRE(messages.warnings.size() == 1); CHECK( test_format_parse_warning(messages.warnings[0]) == - R"(:1:9: warning: Unknown license identifier 'unknownlicense'. Known values are listed at https://spdx.org/licenses/ + R"(: warning: Unknown license identifier 'unknownlicense'. Known values are listed at https://spdx.org/licenses/ on expression: MIT AND unknownlicense ^)"); } diff --git a/src/vcpkg-test/messages.cpp b/src/vcpkg-test/messages.cpp index 842b354e08..c34a2e9bdf 100644 --- a/src/vcpkg-test/messages.cpp +++ b/src/vcpkg-test/messages.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -99,3 +100,387 @@ TEST_CASE ("generate message get_format_arg_mismatches", "[messages]") CHECK(res.arguments_without_comment == std::vector{"go", "ho"}); CHECK(res.comments_without_argument == std::vector{"blah"}); } + +namespace +{ + template + constexpr bool adapt_context_to_expected_invocable_with_impl = false; + + template + constexpr bool adapt_context_to_expected_invocable_with_impl< + std::void_t(), std::declval()...))>, + Test, + Args...> = true; + + template + constexpr bool adapt_context_to_expected_invocable_with = + adapt_context_to_expected_invocable_with_impl; + + static_assert(!adapt_context_to_expected_invocable_with, + "Callable needs to return optional or unique_ptr"); + + // The following tests are the cross product of: + // {prvalue, lvalue, xvalue} X {non-const, const} X {non-ref, ref} + + Optional returns_optional_prvalue(DiagnosticContext&, int val) { return val; } + static_assert(adapt_context_to_expected_invocable_with, "boom"); + static_assert(std::is_same_v, decltype(adapt_context_to_expected(returns_optional_prvalue, 42))>, + "boom"); + + const Optional returns_optional_const_prvalue(DiagnosticContext&, int val) { return val; } + static_assert(adapt_context_to_expected_invocable_with, "boom"); + static_assert( + std::is_same_v, decltype(adapt_context_to_expected(returns_optional_const_prvalue, 42))>, + "boom"); + + Optional& returns_optional_lvalue(DiagnosticContext&, Optional& val) { return val; } + static_assert(adapt_context_to_expected_invocable_with&>, "boom"); + static_assert( + std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_lvalue, std::declval&>()))>, + "boom"); + + const Optional& returns_optional_const_lvalue(DiagnosticContext&, const Optional& val) { return val; } + static_assert( + adapt_context_to_expected_invocable_with&>, + "boom"); + static_assert(std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_const_lvalue, + std::declval&>()))>, + "boom"); + + Optional&& returns_optional_xvalue(DiagnosticContext&, Optional&& val) { return std::move(val); } + static_assert(adapt_context_to_expected_invocable_with>, "boom"); + static_assert( + std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_xvalue, std::declval>()))>, + "boom"); + + const Optional&& returns_optional_const_xvalue(DiagnosticContext&, Optional&& val) + { + return std::move(val); + } + static_assert(adapt_context_to_expected_invocable_with>, + "boom"); + static_assert(std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_const_xvalue, + std::declval>()))>, + "boom"); + + Optional returns_optional_ref_prvalue(DiagnosticContext&, int& val) { return val; } + static_assert(adapt_context_to_expected_invocable_with, "boom"); + static_assert( + std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_ref_prvalue, std::declval()))>, + "boom"); + + const Optional returns_optional_ref_const_prvalue(DiagnosticContext&, int& val) { return val; } + static_assert(adapt_context_to_expected_invocable_with, "boom"); + static_assert( + std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_ref_const_prvalue, std::declval()))>, + "boom"); + + Optional& returns_optional_ref_lvalue(DiagnosticContext&, Optional& val) { return val; } + static_assert(adapt_context_to_expected_invocable_with&>, + "boom"); + static_assert(std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_ref_lvalue, + std::declval&>()))>, + "boom"); + + const Optional& returns_optional_ref_const_lvalue(DiagnosticContext&, const Optional& val) + { + return val; + } + static_assert( + adapt_context_to_expected_invocable_with&>, + "boom"); + static_assert(std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_ref_const_lvalue, + std::declval&>()))>, + "boom"); + + Optional&& returns_optional_ref_xvalue(DiagnosticContext&, Optional&& val) { return std::move(val); } + static_assert(adapt_context_to_expected_invocable_with>, + "boom"); + static_assert(std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_ref_xvalue, + std::declval>()))>, + "boom"); + + const Optional&& returns_optional_ref_const_xvalue(DiagnosticContext&, Optional&& val) + { + return std::move(val); + } + static_assert(adapt_context_to_expected_invocable_with>, + "boom"); + static_assert(std::is_same_v, + decltype(adapt_context_to_expected(returns_optional_ref_const_xvalue, + std::declval>()))>, + "boom"); + + Optional returns_optional_fail(DiagnosticContext& context) + { + context.report({DiagKind::Error, LocalizedString::from_raw("something bad happened")}); + return nullopt; + } + + // The following tests are the cross product of: + // {prvalue, lvalue, xvalue} X {non-const, const} + std::unique_ptr returns_unique_ptr_prvalue(DiagnosticContext&, int val) + { + return std::unique_ptr{new int{val}}; + } + static_assert(adapt_context_to_expected_invocable_with, "boom"); + static_assert(std::is_same_v>, + decltype(adapt_context_to_expected(returns_unique_ptr_prvalue, 42))>, + "boom"); + + using returns_unique_ptr_const_prvalue_t = const std::unique_ptr (*)(DiagnosticContext&, int); + static_assert(!adapt_context_to_expected_invocable_with, "boom"); + + std::unique_ptr& returns_unique_ptr_lvalue(DiagnosticContext&, std::unique_ptr& ret) { return ret; } + static_assert(adapt_context_to_expected_invocable_with&>, + "boom"); + static_assert(std::is_same_v&>, + decltype(adapt_context_to_expected(returns_unique_ptr_lvalue, + std::declval&>()))>, + "boom"); + + const std::unique_ptr& returns_unique_ptr_const_lvalue(DiagnosticContext&, const std::unique_ptr& ret) + { + return ret; + } + static_assert(adapt_context_to_expected_invocable_with&>, + "boom"); + static_assert(std::is_same_v&>, + decltype(adapt_context_to_expected(returns_unique_ptr_const_lvalue, + std::declval&>()))>, + "boom"); + + std::unique_ptr&& returns_unique_ptr_xvalue(DiagnosticContext&, std::unique_ptr&& val) + { + return std::move(val); + } + static_assert(adapt_context_to_expected_invocable_with&&>, + "boom"); + static_assert(std::is_same_v>, + decltype(adapt_context_to_expected(returns_unique_ptr_xvalue, + std::declval>()))>, + "boom"); + + using returns_unique_ptr_const_xvalue_t = const std::unique_ptr && + (*)(DiagnosticContext&, const std::unique_ptr&&); + + static_assert( + !adapt_context_to_expected_invocable_with&&>, + "boom"); + + std::unique_ptr returns_unique_ptr_fail(DiagnosticContext& context) + { + context.report({DiagKind::Error, LocalizedString::from_raw("something bad happened")}); + return nullptr; + } + + struct CopyOnce + { + bool& m_copied; + + explicit CopyOnce(bool& copied) : m_copied(copied) { } + CopyOnce(const CopyOnce& other) : m_copied(other.m_copied) + { + REQUIRE(!m_copied); + m_copied = true; + } + + CopyOnce& operator=(const CopyOnce&) = delete; + }; + + struct MoveCounter + { + int& m_move_limit; + + explicit MoveCounter(int& move_limit) : m_move_limit(move_limit) { } + MoveCounter(const MoveCounter&) = delete; + MoveCounter(MoveCounter&& other) : m_move_limit(other.m_move_limit) + { + REQUIRE(m_move_limit > 0); + --m_move_limit; + } + + MoveCounter& operator=(const MoveCounter&) = delete; + MoveCounter& operator=(MoveCounter&&) = delete; + }; +} // unnamed namespace + +template +bool same_object(T& lhs, U& rhs) +{ + return &lhs == &rhs; +} + +TEST_CASE ("adapt DiagnosticContext to ExpectedL", "[diagnostics]") +{ + // returns_optional_prvalue + { + auto adapted = adapt_context_to_expected(returns_optional_prvalue, 42); + REQUIRE(adapted.value_or_exit(VCPKG_LINE_INFO) == 42); + } + { + int move_limit = 1; + auto adapted = adapt_context_to_expected( + [](DiagnosticContext&, int& move_limit) { + Optional ret; + ret.emplace(move_limit); + return ret; + }, + move_limit); + + REQUIRE(move_limit == 0); + } + // returns_optional_const_prvalue + { + auto adapted = adapt_context_to_expected(returns_optional_const_prvalue, 42); + REQUIRE(adapted.value_or_exit(VCPKG_LINE_INFO) == 42); + } + // returns_optional_lvalue + { + Optional the_lvalue = 42; + auto adapted = adapt_context_to_expected(returns_optional_lvalue, the_lvalue); + REQUIRE(same_object(adapted.value_or_exit(VCPKG_LINE_INFO), the_lvalue.value_or_exit(VCPKG_LINE_INFO))); + } + // returns_optional_const_lvalue + { + Optional the_lvalue = 42; + auto adapted = adapt_context_to_expected(returns_optional_const_lvalue, the_lvalue); + REQUIRE(same_object(adapted.value_or_exit(VCPKG_LINE_INFO), the_lvalue.value_or_exit(VCPKG_LINE_INFO))); + } + // returns_optional_xvalue + { + Optional an_lvalue = 42; + REQUIRE( + adapt_context_to_expected(returns_optional_xvalue, std::move(an_lvalue)).value_or_exit(VCPKG_LINE_INFO) == + 42); + } + { + int move_limit = 1; + Optional an_lvalue; + an_lvalue.emplace(move_limit); + auto adapted = adapt_context_to_expected( + [](DiagnosticContext&, Optional&& ret) -> Optional&& { return std::move(ret); }, + std::move(an_lvalue)); + + REQUIRE(move_limit == 0); + } + // returns_optional_const_xvalue + { + Optional an_lvalue = 42; + REQUIRE(adapt_context_to_expected(returns_optional_const_xvalue, std::move(an_lvalue)) + .value_or_exit(VCPKG_LINE_INFO) == 42); + } + { + bool copied = false; + Optional an_lvalue; + an_lvalue.emplace(copied); + auto adapted = adapt_context_to_expected( + [](DiagnosticContext&, const Optional&& ret) -> const Optional&& { + return std::move(ret); + }, + std::move(an_lvalue)); + + REQUIRE(copied); + } + // returns_optional_ref_prvalue + { + int the_lvalue = 42; + auto adapted = adapt_context_to_expected(returns_optional_ref_prvalue, the_lvalue); + REQUIRE(adapted.get() == &the_lvalue); + } + // returns_optional_ref_const_prvalue + { + int the_lvalue = 42; + auto adapted = adapt_context_to_expected(returns_optional_ref_const_prvalue, the_lvalue); + REQUIRE(adapted.get() == &the_lvalue); + } + // returns_optional_ref_lvalue + { + int the_inside_lvalue = 42; + Optional the_lvalue{the_inside_lvalue}; + auto adapted = adapt_context_to_expected(returns_optional_ref_lvalue, the_lvalue); + REQUIRE(adapted.get() == &the_inside_lvalue); + } + { + int move_limit = 0; + MoveCounter the_inside_lvalue{move_limit}; + Optional an_lvalue; + an_lvalue.emplace(the_inside_lvalue); + auto adapted = adapt_context_to_expected( + [](DiagnosticContext&, Optional&& ret) -> Optional { return std::move(ret); }, + std::move(an_lvalue)); + REQUIRE(adapted.get() == &the_inside_lvalue); + REQUIRE(move_limit == 0); + } + // returns_optional_ref_const_lvalue + { + int the_inside_lvalue = 42; + Optional the_lvalue{the_inside_lvalue}; + auto adapted = adapt_context_to_expected(returns_optional_ref_const_lvalue, the_lvalue); + REQUIRE(adapted.get() == &the_inside_lvalue); + } + // returns_optional_ref_xvalue + { + int the_inside_lvalue = 42; + Optional the_lvalue{the_inside_lvalue}; + auto adapted = adapt_context_to_expected(returns_optional_ref_xvalue, std::move(the_lvalue)); + REQUIRE(adapted.get() == &the_inside_lvalue); + } + // returns_optional_ref_const_xvalue + { + int the_inside_lvalue = 42; + Optional the_lvalue{the_inside_lvalue}; + auto adapted = adapt_context_to_expected(returns_optional_ref_const_xvalue, std::move(the_lvalue)); + REQUIRE(adapted.get() == &the_inside_lvalue); + } + + // returns_optional_prvalue_fail + { + auto adapted = adapt_context_to_expected(returns_optional_fail); + REQUIRE(!adapted.has_value()); + REQUIRE(adapted.error() == LocalizedString::from_raw("error: something bad happened")); + } + + // returns_unique_ptr_prvalue + { + auto adapted = adapt_context_to_expected(returns_unique_ptr_prvalue, 42); + REQUIRE(*(adapted.value_or_exit(VCPKG_LINE_INFO)) == 42); + } + // returns_unique_ptr_lvalue + { + auto an_lvalue = std::make_unique(42); + auto adapted = adapt_context_to_expected(returns_unique_ptr_lvalue, an_lvalue); + REQUIRE(same_object(adapted.value_or_exit(VCPKG_LINE_INFO), an_lvalue)); + } + // returns_unique_ptr_const_lvalue + { + auto an_lvalue = std::make_unique(42); + auto adapted = adapt_context_to_expected(returns_unique_ptr_const_lvalue, an_lvalue); + REQUIRE(same_object(adapted.value_or_exit(VCPKG_LINE_INFO), an_lvalue)); + } + // returns_unique_ptr_xvalue + { + auto an_lvalue = std::make_unique(42); + auto the_pointer = an_lvalue.get(); + auto adapted = adapt_context_to_expected(returns_unique_ptr_xvalue, std::move(an_lvalue)); + REQUIRE(!an_lvalue); // was moved into the adapted + REQUIRE(adapted.value_or_exit(VCPKG_LINE_INFO).get() == the_pointer); + } + + // returns_unique_ptr_fail + { + auto adapted = adapt_context_to_expected(returns_unique_ptr_fail); + REQUIRE(!adapted.has_value()); + REQUIRE(adapted.error() == LocalizedString::from_raw("error: something bad happened")); + } +} diff --git a/src/vcpkg-test/util.cpp b/src/vcpkg-test/util.cpp index 79a1e94504..a419f4ab2f 100644 --- a/src/vcpkg-test/util.cpp +++ b/src/vcpkg-test/util.cpp @@ -128,7 +128,7 @@ namespace vcpkg::Test std::vector parse_test_fspecs(StringView sv) { std::vector ret; - ParserBase parser(sv, "test"); + ParserBase parser(sv, "test", {0, 0}); while (!parser.at_eof()) { auto maybe_opt = parse_qualified_specifier( diff --git a/src/vcpkg/base/diagnostics.cpp b/src/vcpkg/base/diagnostics.cpp new file mode 100644 index 0000000000..2ecb4e8052 --- /dev/null +++ b/src/vcpkg/base/diagnostics.cpp @@ -0,0 +1,198 @@ +#include +#include +#include + +#include + +using namespace vcpkg; + +namespace +{ + static constexpr StringLiteral ColonSpace{": "}; + + void append_file_prefix(std::string& target, const Optional& maybe_origin, const TextRowCol& position) + { + // file:line:col: kind: message + if (auto origin = maybe_origin.get()) + { + target.append(*origin); + if (position.row) + { + target.push_back(':'); + fmt::format_to(std::back_inserter(target), FMT_COMPILE("{}"), position.row); + + if (position.column) + { + target.push_back(':'); + fmt::format_to(std::back_inserter(target), FMT_COMPILE("{}"), position.column); + } + } + + target.append(ColonSpace.data(), ColonSpace.size()); + } + } + + void append_kind_prefix(std::string& target, DiagKind kind) + { + static constexpr StringLiteral Empty{""}; + static constexpr const StringLiteral* prefixes[] = { + &Empty, &MessagePrefix, &ErrorPrefix, &WarningPrefix, &NotePrefix}; + static_assert(std::size(prefixes) == static_cast(DiagKind::COUNT)); + + const auto diag_index = static_cast(kind); + if (diag_index >= static_cast(DiagKind::COUNT)) + { + Checks::unreachable(VCPKG_LINE_INFO); + } + + const auto prefix = prefixes[diag_index]; + target.append(prefix->data(), prefix->size()); + } +} + +namespace vcpkg +{ + void DiagnosticLine::print_to(MessageSink& sink) const + { + std::string buf; + append_file_prefix(buf, m_origin, m_position); + switch (m_kind) + { + case DiagKind::None: + // intentionally blank + break; + case DiagKind::Message: buf.append(MessagePrefix.data(), MessagePrefix.size()); break; + case DiagKind::Error: + { + sink.print(Color::none, buf); + sink.print(Color::error, "error"); + buf.assign(ColonSpace.data(), ColonSpace.size()); + } + break; + case DiagKind::Warning: + { + sink.print(Color::none, buf); + sink.print(Color::warning, "warning"); + buf.assign(ColonSpace.data(), ColonSpace.size()); + } + break; + case DiagKind::Note: buf.append(NotePrefix.data(), NotePrefix.size()); break; + default: Checks::unreachable(VCPKG_LINE_INFO); + } + + buf.append(m_message.data()); + buf.push_back('\n'); + sink.print(Color::none, buf); + } + std::string DiagnosticLine::to_string() const + { + std::string result; + this->to_string(result); + return result; + } + void DiagnosticLine::to_string(std::string& target) const + { + append_file_prefix(target, m_origin, m_position); + append_kind_prefix(target, m_kind); + target.append(m_message.data()); + } + + LocalizedString DiagnosticLine::to_json_reader_string(const std::string& path, const LocalizedString& type) const + { + std::string result; + append_file_prefix(result, m_origin, m_position); + append_kind_prefix(result, m_kind); + result.append(path); + result.append(" ("); + result.append(type.data()); + result.append(") "); + result.append(m_message.data()); + return LocalizedString::from_raw(result); + } + + void BufferedDiagnosticContext::report(const DiagnosticLine& line) { lines.push_back(line); } + + void BufferedDiagnosticContext::report(DiagnosticLine&& line) { lines.push_back(std::move(line)); } + void BufferedDiagnosticContext::print_to(MessageSink& sink) const + { + for (auto&& line : lines) + { + line.print_to(sink); + } + } + // Converts this message into a string + // Prefer print() if possible because it applies color + // Not safe to use in the face of concurrent calls to report() + std::string BufferedDiagnosticContext::to_string() const + { + std::string result; + this->to_string(result); + return result; + } + void BufferedDiagnosticContext::to_string(std::string& target) const + { + auto first = lines.begin(); + const auto last = lines.end(); + if (first == last) + { + return; + } + + for (;;) + { + first->to_string(target); + if (++first == last) + { + return; + } + + target.push_back('\n'); + } + } + + bool BufferedDiagnosticContext::any_errors() const noexcept + { + for (auto&& line : lines) + { + if (line.kind() == DiagKind::Error) + { + return true; + } + } + + return false; + } +} // namespace vcpkg + +namespace +{ + struct ConsoleDiagnosticContext : DiagnosticContext + { + virtual void report(const DiagnosticLine& line) override { line.print_to(out_sink); } + }; + + ConsoleDiagnosticContext console_diagnostic_context_instance; +} // unnamed namespace + +namespace vcpkg +{ + DiagnosticContext& console_diagnostic_context = console_diagnostic_context_instance; +} + +namespace +{ + struct NullDiagnosticContext : DiagnosticContext + { + virtual void report(const DiagnosticLine&) override + { + // intentionally empty + } + }; + + NullDiagnosticContext null_diagnostic_context_instance; +} + +namespace vcpkg +{ + DiagnosticContext& null_diagnostic_context = null_diagnostic_context_instance; +} diff --git a/src/vcpkg/base/git.cpp b/src/vcpkg/base/git.cpp index 9b9ccffb6d..a0be0622ce 100644 --- a/src/vcpkg/base/git.cpp +++ b/src/vcpkg/base/git.cpp @@ -86,7 +86,7 @@ namespace vcpkg }; std::vector results; - ParserBase parser(output, "git status"); + ParserBase parser(output, "git status", {0, 0}); while (!parser.at_eof()) { GitStatusLine result; diff --git a/src/vcpkg/base/json.cpp b/src/vcpkg/base/json.cpp index 2c7116d117..bab3ccff65 100644 --- a/src/vcpkg/base/json.cpp +++ b/src/vcpkg/base/json.cpp @@ -544,7 +544,10 @@ namespace vcpkg::Json { struct Parser : private ParserBase { - Parser(StringView text, StringView origin) : ParserBase(text, origin), style_() { } + Parser(StringView text, StringView origin, TextRowCol init_rowcol) + : ParserBase(text, origin, init_rowcol), style_() + { + } char32_t next() noexcept { @@ -1059,7 +1062,7 @@ namespace vcpkg::Json { StatsTimer t(g_json_parsing_stats); - auto parser = Parser(json, origin); + auto parser = Parser(json, origin, {1, 1}); auto val = parser.parse_value(); diff --git a/src/vcpkg/base/parse.cpp b/src/vcpkg/base/parse.cpp index b1ded452c1..58b4dcc3dc 100644 --- a/src/vcpkg/base/parse.cpp +++ b/src/vcpkg/base/parse.cpp @@ -9,8 +9,19 @@ namespace vcpkg { static void advance_rowcol(char32_t ch, int& row, int& column) { + if (row == 0 && column == 0) + { + return; + } + else if (row == 0 || column == 0) + { + Checks::unreachable(VCPKG_LINE_INFO); + } + if (ch == '\t') + { column = ((column + 7) & ~7) + 1; // round to next 8-width tab stop + } else if (ch == '\n') { row++; @@ -69,7 +80,14 @@ namespace vcpkg LocalizedString res; if (!origin.empty()) { - res.append_raw(fmt::format("{}:{}:{}: ", origin, location.row, location.column)); + if (location.row == 0 && location.column == 0) + { + res.append_raw(fmt::format("{}: ", origin)); + } + else + { + res.append_raw(fmt::format("{}:{}:{}: ", origin, location.row, location.column)); + } } res.append_raw(kind == MessageKind::Warning ? WarningPrefix : ErrorPrefix); @@ -100,8 +118,8 @@ namespace vcpkg ParserBase::ParserBase(StringView text, Optional origin, TextRowCol init_rowcol) : m_it(text.begin(), text.end()) , m_start_of_line(m_it) - , m_row(init_rowcol.row_or(1)) - , m_column(init_rowcol.column_or(1)) + , m_row(init_rowcol.row) + , m_column(init_rowcol.column) , m_text(text) , m_origin(origin) { @@ -163,7 +181,11 @@ namespace vcpkg // success m_it = encoded; - m_column += static_cast(text.size()); + if (m_column != 0) + { + m_column += static_cast(text.size()); + } + return true; } @@ -189,7 +211,11 @@ namespace vcpkg // success m_it = encoded; - m_column += static_cast(keyword_content.size()); + if (m_column != 0) + { + m_column += static_cast(keyword_content.size()); + } + return true; } @@ -226,7 +252,14 @@ namespace vcpkg auto& res = m_messages.error.emplace(); if (auto origin = m_origin.get()) { - res.append_raw(fmt::format("{}:{}:{}: ", *origin, loc.row, loc.column)); + if (loc.row == 0 && loc.column == 0) + { + res.append_raw(fmt::format("{}: ", *origin)); + } + else + { + res.append_raw(fmt::format("{}:{}:{}: ", *origin, loc.row, loc.column)); + } } res.append_raw(ErrorPrefix); diff --git a/src/vcpkg/base/system.mac.cpp b/src/vcpkg/base/system.mac.cpp index 4543006161..6c6541f7f4 100644 --- a/src/vcpkg/base/system.mac.cpp +++ b/src/vcpkg/base/system.mac.cpp @@ -91,7 +91,7 @@ namespace vcpkg // "connection name","network adapter","physical address","transport name" auto is_quote = [](auto ch) -> bool { return ch == '"'; }; - auto parser = ParserBase(line, "getmac ouptut"); + auto parser = ParserBase(line, "getmac output", {0, 0}); out.clear(); diff --git a/src/vcpkg/base/system.process.cpp b/src/vcpkg/base/system.process.cpp index 70a4efdb89..ffff5d17d6 100644 --- a/src/vcpkg/base/system.process.cpp +++ b/src/vcpkg/base/system.process.cpp @@ -267,7 +267,7 @@ namespace vcpkg Optional try_parse_process_stat_file(const FileContents& contents) { - ParserBase p(contents.content, contents.origin); + ParserBase p(contents.content, contents.origin, {1, 1}); p.match_while(ParserBase::is_ascii_digit); // pid %d (ignored) diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index 93207d06cd..b626ba1c33 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -1455,7 +1455,7 @@ namespace struct BinaryConfigParser : ConfigSegmentsParser { BinaryConfigParser(StringView text, Optional origin, BinaryConfigParserState* state) - : ConfigSegmentsParser(text, origin), state(state) + : ConfigSegmentsParser(text, origin, {0, 0}), state(state) { } @@ -1907,7 +1907,7 @@ namespace struct AssetSourcesParser : ConfigSegmentsParser { AssetSourcesParser(StringView text, StringView origin, AssetSourcesState* state) - : ConfigSegmentsParser(text, origin), state(state) + : ConfigSegmentsParser(text, origin, {0, 0}), state(state) { } diff --git a/src/vcpkg/cgroup-parser.cpp b/src/vcpkg/cgroup-parser.cpp index c3f930c5ba..552b9c7d80 100644 --- a/src/vcpkg/cgroup-parser.cpp +++ b/src/vcpkg/cgroup-parser.cpp @@ -28,7 +28,7 @@ namespace vcpkg using P = ParserBase; constexpr auto is_separator_or_lineend = [](auto ch) { return ch == ':' || P::is_lineend(ch); }; - ParserBase parser{text, origin}; + ParserBase parser{text, origin, {1, 1}}; parser.skip_whitespace(); std::vector ret; diff --git a/src/vcpkg/ci-baseline.cpp b/src/vcpkg/ci-baseline.cpp index a6dc1453a7..a0b312bccd 100644 --- a/src/vcpkg/ci-baseline.cpp +++ b/src/vcpkg/ci-baseline.cpp @@ -59,7 +59,7 @@ namespace vcpkg std::vector parse_ci_baseline(StringView text, StringView origin, ParseMessages& messages) { std::vector result; - ParserBase parser(text, origin); + ParserBase parser(text, origin, {1, 1}); for (;;) { parser.skip_whitespace(); diff --git a/src/vcpkg/commands.package-info.cpp b/src/vcpkg/commands.package-info.cpp index bd94a6235a..55de41ce8d 100644 --- a/src/vcpkg/commands.package-info.cpp +++ b/src/vcpkg/commands.package-info.cpp @@ -106,7 +106,7 @@ namespace vcpkg for (auto&& arg : options.command_arguments) { - ParserBase parser(arg, nullopt); + ParserBase parser(arg, nullopt, {0, 0}); auto maybe_pkg = parse_package_name(parser); if (!parser.at_eof() || !maybe_pkg) { diff --git a/src/vcpkg/packagespec.cpp b/src/vcpkg/packagespec.cpp index 53030973c0..753aba6052 100644 --- a/src/vcpkg/packagespec.cpp +++ b/src/vcpkg/packagespec.cpp @@ -172,7 +172,7 @@ namespace vcpkg AllowPlatformSpec allow_platform_spec) { // there is no origin because this function is used for user inputs - auto parser = ParserBase(input, nullopt); + auto parser = ParserBase(input, nullopt, {0, 0}); auto maybe_pqs = parse_qualified_specifier(parser, allow_features, parse_explicit_triplet, allow_platform_spec); if (!parser.at_eof()) { diff --git a/src/vcpkg/paragraphs.cpp b/src/vcpkg/paragraphs.cpp index e203799b16..e9af529c2e 100644 --- a/src/vcpkg/paragraphs.cpp +++ b/src/vcpkg/paragraphs.cpp @@ -268,7 +268,7 @@ namespace vcpkg::Paragraphs } public: - PghParser(StringView text, StringView origin) : ParserBase(text, origin) { } + PghParser(StringView text, StringView origin) : ParserBase(text, origin, {1, 1}) { } ExpectedL> get_paragraphs() { diff --git a/src/vcpkg/platform-expression.cpp b/src/vcpkg/platform-expression.cpp index 8441563c36..73c39d8128 100644 --- a/src/vcpkg/platform-expression.cpp +++ b/src/vcpkg/platform-expression.cpp @@ -119,7 +119,7 @@ namespace vcpkg::PlatformExpression struct ExpressionParser : ParserBase { ExpressionParser(StringView str, MultipleBinaryOperators multiple_binary_operators) - : ParserBase(str, "CONTROL"), multiple_binary_operators(multiple_binary_operators) + : ParserBase(str, "CONTROL", {0, 0}), multiple_binary_operators(multiple_binary_operators) { } diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 1a581431ab..a250c4c4b6 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -753,7 +753,7 @@ namespace vcpkg // * `null`, for when the license of the package cannot be described by an SPDX expression struct SpdxLicenseExpressionParser : ParserBase { - SpdxLicenseExpressionParser(StringView sv, StringView origin) : ParserBase(sv, origin) { } + SpdxLicenseExpressionParser(StringView sv, StringView origin) : ParserBase(sv, origin, {0, 0}) { } static const StringLiteral* case_insensitive_find(View lst, StringView id) {