In several PRs, such as microsoft#908 and microsoft#1210 , it has bec…

…ome clear that we need a thread safe channel through which errors and warnings can both be reasonably reported. Now that microsoft#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<T> example_api(int a);

    ExpectedL<std::unique_ptr<SourceControlFile>> 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<T> example_api(MessageContext& context, int a);

    // unique_ptr is already 'optional'
    std::unique_ptr<SourceControlFile> 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 ( microsoft#1290 ) or complex enough that we are not sure they are correct by trying to recover boundaries by reparsing our own error output ( microsoft#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

@@ -0,0 +1,58 @@
+#pragma once
+
+#include <vcpkg/base/messages.h>
+#include <vcpkg/base/optional.h>
+
+#include <string>
+
+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
+    };
+
+    struct TextPosition
+    {
+        // '0' indicates uninitialized; '1' is the first row/column
+        int row = 0;
+        int column = 0;
+    };
+
+    struct DiagnosticLine
+    {
+        DiagnosticLine(MessageKind kind, LocalizedString&& message)
+            : m_kind(kind), m_origin(), m_position(), m_message(std::move(message))
+        {
+        }
+
+        DiagnosticLine(MessageKind kind, StringView origin, LocalizedString&& message)
+            : m_kind(kind), m_origin(origin.to_string()), m_position(), m_message(std::move(message))
+        {
+        }
+
+        DiagnosticLine(MessageKind kind, StringView origin, TextPosition position, LocalizedString&& message)
+            : m_kind(kind), m_origin(origin.to_string()), m_position(position), m_message(std::move(message))
+        {
+        }
+
+    private:
+        MessageKind m_kind;
+        Optional<std::string> m_origin;
+        TextPosition m_position;
+        LocalizedString m_message;
+    };
+
+    struct DiagnosticContext
+    {
+        virtual void report(const DiagnosticLine& line);
+        virtual void report(DiagnosticLine&& line) { report(line); }
+
+    protected:
+        ~DiagnosticContext() = default;
+    };
+}

include/vcpkg/base/parse.h

@@ -2,13 +2,12 @@
 
 #include <vcpkg/base/fwd/parse.h>
 
+#include <vcpkg/base/diagnostics.h>
 #include <vcpkg/base/messages.h>
 #include <vcpkg/base/optional.h>
 #include <vcpkg/base/stringview.h>
 #include <vcpkg/base/unicode.h>
 
-#include <vcpkg/textrowcol.h>
-
 #include <memory>
 #include <string>
 
@@ -75,7 +74,7 @@ namespace vcpkg
 
     struct ParserBase
     {
-        ParserBase(StringView text, StringView origin, TextRowCol init_rowcol = {});
+        ParserBase(StringView text, StringView origin, TextPosition init_rowcol = {1, 1});
 
         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'; }
@@ -131,7 +130,7 @@ namespace vcpkg
         Unicode::Utf8Decoder it() const { return m_it; }
         char32_t cur() const { return m_it == m_it.end() ? Unicode::end_of_file : *m_it; }
         SourceLoc cur_loc() const { return {m_it, m_start_of_line, m_row, m_column}; }
-        TextRowCol cur_rowcol() const { return {m_row, m_column}; }
+        TextPosition cur_rowcol() const { return {m_row, m_column}; }
         char32_t next();
         bool at_eof() const { return m_it == m_it.end(); }
 

include/vcpkg/paragraphparser.h

@@ -2,12 +2,12 @@
 
 #include <vcpkg/fwd/paragraphparser.h>
 
+#include <vcpkg/base/diagnostics.h>
 #include <vcpkg/base/expected.h>
 #include <vcpkg/base/messages.h>
 #include <vcpkg/base/stringview.h>
 
 #include <vcpkg/packagespec.h>
-#include <vcpkg/textrowcol.h>
 
 #include <map>
 #include <memory>
@@ -16,7 +16,7 @@
 
 namespace vcpkg
 {
-    using Paragraph = std::map<std::string, std::pair<std::string, TextRowCol>, std::less<>>;
+    using Paragraph = std::map<std::string, std::pair<std::string, TextPosition>, std::less<>>;
 
     struct ParagraphParser
     {
@@ -28,9 +28,9 @@ namespace vcpkg
         std::string required_field(StringLiteral fieldname);
 
         std::string optional_field(StringLiteral fieldname);
-        std::string optional_field(StringLiteral fieldname, TextRowCol& position);
+        std::string optional_field(StringLiteral fieldname, TextPosition& position);
 
-        void add_error(TextRowCol position, msg::MessageT<> error_content);
+        void add_error(TextPosition position, msg::MessageT<> error_content);
 
         Optional<LocalizedString> error() const;
 
@@ -42,8 +42,8 @@ namespace vcpkg
 
     ExpectedL<std::vector<std::string>> parse_default_features_list(const std::string& str,
                                                                     StringView origin = "<unknown>",
-                                                                    TextRowCol textrowcol = {});
+                                                                    TextPosition position = {1, 1});
     ExpectedL<std::vector<ParsedQualifiedSpecifier>> parse_qualified_specifier_list(const std::string& str,
                                                                                     StringView origin = "<unknown>",
-                                                                                    TextRowCol textrowcol = {});
+                                                                                    TextPosition position = {1, 1});
 }

include/vcpkg/sourceparagraph.h

@@ -223,5 +223,5 @@ namespace vcpkg
     // Exposed for testing
     ExpectedL<std::vector<Dependency>> parse_dependencies_list(const std::string& str,
                                                                StringView origin,
-                                                               TextRowCol textrowcol = {});
+                                                               TextPosition position = {1, 1});
 }

src/vcpkg-test/paragraph.cpp

@@ -15,7 +15,7 @@ namespace
         {
             pghs.emplace_back();
             for (auto&& kv : p)
-                pghs.back().emplace(kv.first, std::make_pair(kv.second, vcpkg::TextRowCol{}));
+                pghs.back().emplace(kv.first, std::make_pair(kv.second, vcpkg::TextPosition{}));
         }
         return vcpkg::SourceControlFile::parse_control_file("", std::move(pghs));
     }
@@ -24,7 +24,7 @@ namespace
     {
         Paragraph pgh;
         for (auto&& kv : v)
-            pgh.emplace(kv.first, std::make_pair(kv.second, vcpkg::TextRowCol{}));
+            pgh.emplace(kv.first, std::make_pair(kv.second, vcpkg::TextPosition{}));
 
         return vcpkg::BinaryParagraph("test", std::move(pgh));
     }

src/vcpkg-test/util.cpp

@@ -63,7 +63,7 @@ namespace vcpkg::Test
             pghs.emplace_back();
             for (auto&& kv : p)
             {
-                pghs.back().emplace(kv.first, std::make_pair(kv.second, vcpkg::TextRowCol{}));
+                pghs.back().emplace(kv.first, std::make_pair(kv.second, vcpkg::TextPosition{}));
             }
         }
         return vcpkg::SourceControlFile::parse_control_file("", std::move(pghs));

src/vcpkg/base/parse.cpp

@@ -90,11 +90,11 @@ namespace vcpkg
         }
     }
 
-    ParserBase::ParserBase(StringView text, StringView origin, TextRowCol init_rowcol)
+    ParserBase::ParserBase(StringView text, StringView origin, TextPosition 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)
     {

src/vcpkg/binaryparagraph.cpp

@@ -35,7 +35,7 @@ namespace vcpkg
 
         // one or the other
         this->version.text = parser.optional_field(Fields::VERSION);
-        TextRowCol pv_position;
+        TextPosition pv_position;
         auto pv_str = parser.optional_field(Fields::PORT_VERSION, pv_position);
         this->version.port_version = 0;
         if (!pv_str.empty())

src/vcpkg/paragraphs.cpp

@@ -20,7 +20,7 @@ static std::atomic<uint64_t> g_load_ports_stats(0);
 
 namespace vcpkg
 {
-    static Optional<std::pair<std::string, TextRowCol>> remove_field(Paragraph* fields, StringView fieldname)
+    static Optional<std::pair<std::string, TextPosition>> remove_field(Paragraph* fields, StringView fieldname)
     {
         auto it = fields->find(fieldname.to_string());
         if (it == fields->end())
@@ -33,7 +33,7 @@ namespace vcpkg
         return value;
     }
 
-    std::string ParagraphParser::optional_field(StringLiteral fieldname, TextRowCol& position)
+    std::string ParagraphParser::optional_field(StringLiteral fieldname, TextPosition& position)
     {
         auto maybe_field = remove_field(&fields, fieldname);
         if (auto field = maybe_field.get())
@@ -47,7 +47,7 @@ namespace vcpkg
 
     std::string ParagraphParser::optional_field(StringLiteral fieldname)
     {
-        TextRowCol ignore;
+        TextPosition ignore;
         return optional_field(fieldname, ignore);
     }
 
@@ -66,7 +66,7 @@ namespace vcpkg
         return std::string();
     }
 
-    void ParagraphParser::add_error(TextRowCol position, msg::MessageT<> error_content)
+    void ParagraphParser::add_error(TextPosition position, msg::MessageT<> error_content)
     {
         errors.emplace_back(LocalizedString::from_raw(origin)
                                 .append_raw(fmt::format("{}:{}: ", position.row, position.column))
@@ -167,18 +167,18 @@ namespace vcpkg
 
     ExpectedL<std::vector<std::string>> parse_default_features_list(const std::string& str,
                                                                     StringView origin,
-                                                                    TextRowCol textrowcol)
+                                                                    TextPosition position)
     {
-        auto parser = ParserBase(str, origin, textrowcol);
+        auto parser = ParserBase(str, origin, position);
         auto opt = parse_list_until_eof<std::string>(msgExpectedDefaultFeaturesList, parser, &parse_feature_name);
         if (!opt) return {LocalizedString::from_raw(parser.get_error()->to_string()), expected_right_tag};
         return {std::move(opt).value_or_exit(VCPKG_LINE_INFO), expected_left_tag};
     }
     ExpectedL<std::vector<ParsedQualifiedSpecifier>> parse_qualified_specifier_list(const std::string& str,
                                                                                     StringView origin,
-                                                                                    TextRowCol textrowcol)
+                                                                                    TextPosition position)
     {
-        auto parser = ParserBase(str, origin, textrowcol);
+        auto parser = ParserBase(str, origin, position);
         auto opt = parse_list_until_eof<ParsedQualifiedSpecifier>(
             msgExpectedDependenciesList, parser, [](ParserBase& parser) { return parse_qualified_specifier(parser); });
         if (!opt) return {LocalizedString::from_raw(parser.get_error()->to_string()), expected_right_tag};
@@ -187,9 +187,9 @@ namespace vcpkg
     }
     ExpectedL<std::vector<Dependency>> parse_dependencies_list(const std::string& str,
                                                                StringView origin,
-                                                               TextRowCol textrowcol)
+                                                               TextPosition position)
     {
-        auto parser = ParserBase(str, origin, textrowcol);
+        auto parser = ParserBase(str, origin, position);
         auto opt = parse_list_until_eof<Dependency>(msgExpectedDependenciesList, parser, [](ParserBase& parser) {
             auto loc = parser.cur_loc();
             return parse_qualified_specifier(parser).then([&](ParsedQualifiedSpecifier&& pqs) -> Optional<Dependency> {

src/vcpkg/sourceparagraph.cpp

@@ -300,7 +300,7 @@ namespace vcpkg
         spgh->name = parser.required_field(SourceParagraphFields::NAME);
         spgh->version.text = parser.required_field(SourceParagraphFields::VERSION);
 
-        TextRowCol pv_position;
+        TextPosition pv_position;
         auto pv_str = parser.optional_field(SourceParagraphFields::PORT_VERSION, pv_position);
         if (!pv_str.empty())
         {
@@ -322,10 +322,10 @@ namespace vcpkg
         trim_all(spgh->maintainers);
 
         spgh->homepage = parser.optional_field(SourceParagraphFields::HOMEPAGE);
-        TextRowCol textrowcol;
-        std::string buf = parser.optional_field(SourceParagraphFields::BUILD_DEPENDS, textrowcol);
+        TextPosition build_depends_position;
+        std::string buf = parser.optional_field(SourceParagraphFields::BUILD_DEPENDS, build_depends_position);
 
-        auto maybe_dependencies = parse_dependencies_list(buf, origin, textrowcol);
+        auto maybe_dependencies = parse_dependencies_list(buf, origin, build_depends_position);
         if (const auto dependencies = maybe_dependencies.get())
         {
             spgh->dependencies = *dependencies;
@@ -335,9 +335,10 @@ namespace vcpkg
             return std::move(maybe_dependencies).error();
         }
 
-        buf = parser.optional_field(SourceParagraphFields::DEFAULT_FEATURES, textrowcol);
+        TextPosition default_features_position;
+        buf = parser.optional_field(SourceParagraphFields::DEFAULT_FEATURES, default_features_position);
 
-        auto maybe_default_features = parse_default_features_list(buf, origin, textrowcol);
+        auto maybe_default_features = parse_default_features_list(buf, origin, default_features_position);
         if (const auto default_features = maybe_default_features.get())
         {
             for (auto&& default_feature : *default_features)
@@ -368,7 +369,7 @@ namespace vcpkg
             return std::move(maybe_default_features).error();
         }
 
-        TextRowCol supports_position;
+        TextPosition supports_position;
         auto supports_text = parser.optional_field(SourceParagraphFields::SUPPORTS, supports_position);
         if (!supports_text.empty())
         {