Introduce DiagnosticContext, an 'error document' like type. #1323

BillyONeal · 2024-01-10T06:04:48Z

In several PRs, such as #908 and #1210 , it has become clear that we need a thread safe channel through which errors and warnings can both be reasonably reported. Now that #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:

    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.

Distinguish whether an overall operation succeeded or failed in the return value, but
record any errors or warnings via an out parameter.

Applying this to the above gives:

    Optional<T> example_api(DiagnosticContext& context, int a);

    // unique_ptr is already 'optional'
    std::unique_ptr<SourceControlFile> try_load_port_manifest_text(DiagnosticContext& 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 ( Make individual print calls thread safe #1290 ) or complex enough that we are not sure they are correct by trying to recover boundaries by reparsing our own error output ( Binary cache: async push_success #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.

Unblocks:

Install files in parallel #1256 (or at least work on it)
Rework and parallelize abi hashing #1225 (though I am still not convinced that this change should happen as 'rework' needs serious justification not present in the PR)
Teach x-ci-verify-versions to check that versions exist in the database #1210 (resolves code review comment about hating multiple return channels for errors and warnings)
Binary cache: async push_success #908

include/vcpkg/base/diagnostics.h

include/vcpkg/base/parse.h

src/vcpkg-test/ci-baseline.cpp

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

…erence-collapsing.

BillyONeal · 2024-01-27T00:03:12Z

@autoantwort What do you think of this direction?

autoantwort · 2024-01-27T08:16:30Z

LGTM.

Optional<T> example_api(MessageContext& context, int a);

I don't know if this can get annoying if you have to declare a MessageContext object before each function call (Don't know if this would happen in real code).
If this gets annoying maybe something like OptionalWithMessages<T> example_api(int a); could work (the message part is always there even if there is an object)

autoantwort · 2024-01-27T08:19:06Z

Ah MessageContext/DiagnosticContext is like a MessageSink and would directly print to stdout instead of ExpectedL where the caller has to do that.

BillyONeal · 2024-01-29T20:13:32Z

Ah MessageContext/DiagnosticContext is like a MessageSink and would directly print to stdout instead of ExpectedL where the caller has to do that.

That's right, it's like MessageSink but the granularity is 'whole diagnostic' rather than 'individual prints' meaning it works for the synchronization boundary several PRs are looking for here.

ras0219-msft · 2024-02-14T19:18:13Z

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.

I think this proposal could still handle this well by viewing the DiagnosticContext as the localized, user-facing content and then using an ExpectedT<T, std::error_code> return (or other such machine-relevant error type).

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.

I think this interface could handle this via layering. Just as some top of mind thoughts:

Optional<int> foo(Ctx& ctx) {
   WarningsAsErrorsCtx inner_ctx(ctx);
   auto maybe_bar = bar(inner_ctx, 123);
   if (auto b = maybe_bar.get()) { return *b + 1; }
   return nullopt;
}

Or, when alternatives should be attempted:

Optional<int> foo(Ctx& ctx) {
   BufferCtx buffer_ctx(ctx);
   auto maybe_bar = bar(buffer_ctx, 123);
   if (auto b = maybe_bar.get()) { return *b + 1; }
   else if (/* something else */) {
     /* something else worked, drop the messages */
     buffer_ctx.drop();
     /* or perhaps convert them to warnings? */
     buffer_ctx.errors_to_warnings();
     return 5;
   }
   /* something else failed; buffer_ctx will release messages "normally" */
   return nullopt;
}

ras0219-msft · 2024-02-14T19:07:12Z

include/vcpkg/base/diagnostics.h

+        Optional<std::string> m_origin;
+        TextRowCol m_position;


Would it make sense to bundle these (origin + position) into a single Optional? Alternatively: it's a bit unclear to me what the intended states are.

No origin, No position

Empty-string origin, No Position

Origin, No Position

Empty-string origin, Position

Origin, Position

Empty string origin is never intended. Position without origin is never intended either. I'll look to clarify.

ras0219-msft · 2024-02-14T19:08:26Z

include/vcpkg/base/diagnostics.h

+
+    struct DiagnosticLine
+    {
+        template<class MessageLike, std::enable_if_t<std::is_convertible_v<MessageLike, LocalizedString>, int> = 0>


Since these constructors all have different arities, is this SFINAE needed?

It's needed to make is_constructible_v<DiagnosticLine, DiagKind, int> be false.

(Perfect forwarding is psycho greedy so it basically always needs SFINAE)

What's the concrete consequence of is_constructible_v being true in this case? Issues with wrapping this type inside Expected<>?

Conversely, what value/perf is provided by making this a template versus just taking a LocalizedString?

include/vcpkg/base/diagnostics.h

ras0219-msft · 2024-02-14T19:36:29Z

include/vcpkg/base/diagnostics.h

+        }
+
+        template<class MessageLike, std::enable_if_t<std::is_convertible_v<MessageLike, LocalizedString>, int> = 0>
+        DiagnosticLine(DiagKind kind, StringView origin, TextRowCol position, MessageLike&& message)


I have a thought, not yet a suggestion, based on wild prediction of what this will look like in practice:

This design requires origin and position to be separately plumbed through every function call layer down to where the DiagnosticLine is created -- typically very close to the leaf function that has the appropriate context. I'm afraid many, many functions will end up looking like the following even though they themselves never create a DiagnosticLine -- they just want to call other functions!

Optional<std::string> do_the_thing( DiagnosticContext& ctx, Optional<StringView> origin, TextRowCol position, /* Now I can start talking about _what I actually care about_ */ int a, int b);

To me, this sounds like a lot of boilerplate that (gut feeling) most layers don't care about. Would it make sense to have a source position "stack-y" side channel as part of the context convention?

Optional<std::string> do_the_thing_from_the_file(DiagnosticContext& ctx, Path p, const Filesystem& fs) { if (auto content = fs.read_contents(ctx, p)) { DiagnosticFrame with_pos(ctx, p, TextRowCol{}); /* Look ma, short calling conventions! */ return do_the_thing(ctx, content); } return nullopt; }

This would necessitate 3 different "reporting" overloads:

I know this DiagnosticLine is attached to this specific source position

I know this DiagnosticLine is not attached to any source position

I want to inherit the contextual source position

This design requires origin and position to be separately plumbed through every function call layer down to where the DiagnosticLine is created -- typically very close to the leaf function that has the appropriate context. I'm afraid many, many functions will end up looking like the following even though they themselves never create a DiagnosticLine -- they just want to call other functions!
[...]
/* Now I can start talking about _what I actually care about_ */

I argue that for functions that are talking about file / position information like this, position information is not 'something they don't care about', it's essential in implementing what that function does.

I can see an argument for a type that contains:

A DiagnosticContext&

The origin

Tracks a cursor of where the currently being parsed position is

and today the type trying to do that is ParserBase. I'm not a fan of how ParserBase wants and encourages deriving from and making things members when I think a struct containing these 3 members being passed around is a more composable design. I didn't go all the way there here though because

That starts to approach the design where callers are expected to provide additional wrapping context I asked for but you have said over time IRL that you do not like, and

That would require making this PR bigger since it would change more about how ParserBase is used.

Seeing that ParserBase tracks state incorrectly since it accepted a column but set m_start_of_line(m_it) (so the only valid col was 0 or 1) I've become convinced that 2+3 should go together but 1 should not.

It would be great to add at least a comment outlining the different valid states of this object for future readers. Presumably this should be derivable by looking at all the constructor overloads, but that's much more complex than a single comment block that stands alone.

include/vcpkg/packagespec.h

include/vcpkg/paragraphparser.h

ras0219-msft · 2024-02-14T19:54:08Z

src/vcpkg-test/cgroup-parser.cpp

@@ -72,7 +72,7 @@ TEST_CASE ("parse proc/pid/stat file", "[cgroup-parser]")
        std::string contents =
            R"(4281 (cpptools-srv) S 4099 1676 1676 0 -1 1077936384 51165 303 472 0 81 25 0 0 20 0 10 0 829158 4924583936 39830 18446744073709551615 4194304 14147733 140725993620736 0 0 0 0 16781312 16386 0 0 0 17 1 0 0 5 0 0 16247120 16519160 29999104 140725993622792 140725993622920 140725993622920 140725993627556 0)";

-        auto maybe_stat = try_parse_process_stat_file({contents, "test"});
+        auto maybe_stat = try_parse_process_stat_file(console_diagnostic_context, {contents, "test"});


I believe it should be possible to create a context emitter that interoperates with Catch2 to only display messages on failure.

(I know there's an INFO() macro, but I believe it's stack based)

I don't know of a good way of doing this. The problem with INFO() et al. is that they destroy contextual information about why something happened.

I could see changing these to null_diagnostic_context if you wish?

src/vcpkg-test/manifests.cpp

src/vcpkg/base/diagnostics.cpp

# Conflicts: # azure-pipelines/end-to-end-tests-dir/cli.ps1 # include/vcpkg/base/parse.h # include/vcpkg/input.h # include/vcpkg/packagespec.h # include/vcpkg/paragraphparser.h # include/vcpkg/sourceparagraph.h # src/vcpkg/base/parse.cpp # src/vcpkg/binarycaching.cpp # src/vcpkg/binaryparagraph.cpp # src/vcpkg/commands.build-external.cpp # src/vcpkg/commands.build.cpp # src/vcpkg/commands.check-support.cpp # src/vcpkg/commands.depend-info.cpp # src/vcpkg/commands.export.cpp # src/vcpkg/commands.install.cpp # src/vcpkg/commands.package-info.cpp # src/vcpkg/commands.remove.cpp # src/vcpkg/commands.set-installed.cpp # src/vcpkg/commands.upgrade.cpp # src/vcpkg/input.cpp # src/vcpkg/packagespec.cpp # src/vcpkg/paragraphs.cpp # src/vcpkg/sourceparagraph.cpp

autoantwort · 2024-04-20T18:05:34Z

After in person discussion with @ras0219-msft , he is unhappy with binary caching or similar displaying anything while port builds are happening, so this isn't as natural a synchronization barrier as I thought. I still think this will be helpful in implementing what those other PRs wanted to do but it won't do it entirely.

I thought this was the reason for this PR. Because of robert's concerns BGMessageSink was introduced to prevent interleaved output. But we don't know when a message begins and ends with MessageSinks which would be resolved by the "error document" wrapping the message in an object

BillyONeal · 2024-04-22T21:19:48Z

I thought this was the reason for this PR. Because of robert's concerns BGMessageSink was introduced to prevent interleaved output. But we don't know when a message begins and ends with MessageSinks which would be resolved by the "error document" wrapping the message in an object

I think that's still true. The effect of the discussion there was that implementers of the interface
don't need to make it safe to submit from arbitrary threads. In your case, a context gets made for the BG thread and only the BG thread ever writes to it, so the interface doesn't need to allow interleaved access from threads.

…eculative parsing. Note that TextRowCol was downgraded to int because ParserBase::ParserBase assumes start_of_line == the content.

Thomas1664 · 2024-04-28T23:11:04Z

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 ( Make individual print calls thread safe #1290 )

@BillyONeal I don't think the approach in #1290 is wrong because that is exactly what's needed. Without that PR, you can't even print single messages from multiple threads. We need both PRs!

If I understand this PR correctly, the approach here is to collect all errors and print them when the operation is finished. I think this is more complicated than it needs to be for simple cases like #1256.

autoantwort

I thing we need documentation when one should use msg::println_error, MessageSink::println_error or DiagnosticContext::report_error

autoantwort · 2024-04-30T13:50:32Z

include/vcpkg/base/diagnostics.h

+        std::vector<DiagnosticLine> lines;
+
+        // Prints all diagnostics to the terminal.
+        void print(MessageSink& sink) const;


Suggested change

void print(MessageSink& sink) const;

void print_to(MessageSink& sink) const;

or print_diagnostics_to makes it more clear what this does.

autoantwort · 2024-04-30T13:54:07Z

src/vcpkg/base/diagnostics.cpp

+
+        buf.append(m_message.data());
+        buf.push_back('\n');
+        sink.print(Color::none, buf);


Is there a specific reason why only the error is printed in red and not the whole error message as it is now?

We've gotten feedback in the past that users find our error messages annoying and/or difficult to read because their terminal is bleeding. Coloring only the 'error' or 'warning' part is consistent with what compilers do:

autoantwort · 2024-04-30T13:57:13Z

include/vcpkg/base/diagnostics.h

+        int column = 0;
+    };
+
+    struct DiagnosticLine


Maybe DiagnosticMessage or simple Message? Since it is allowed that the diagnostic line contains multiple lines

The word 'message' is heavily used already by the localization infrastructure and I didn't want to step on such names.

BillyONeal · 2024-05-01T01:10:14Z

I thing we need documentation when one should use msg::println_error, MessageSink::println_error or DiagnosticContext::report_error

msg:: vs DiagnosticContext: Unfortunately I think that one will remain at 'it depends' for a long time. Ideally over time I expect more things will choose the latter but we don't want to set a standard requiring people to upgrade swaths of code in order to do anything.

MessageSink -> DiagnosticContext SoonTM

@ras0219-msft

…1323 as requested by @ras0219-msft

# Conflicts: # include/vcpkg/base/message-data.inc.h # locales/messages.json # src/vcpkg/base/json.cpp # src/vcpkg/commands.ci-verify-versions.cpp # src/vcpkg/commands.format-manifest.cpp # src/vcpkg/paragraphs.cpp

ras0219-msft · 2024-05-01T20:01:26Z

include/vcpkg/base/diagnostics.h

+        }
+
+        template<class MessageLike, std::enable_if_t<std::is_convertible_v<MessageLike, LocalizedString>, int> = 0>
+        DiagnosticLine(DiagKind kind, StringView origin, TextRowCol position, MessageLike&& message)


It would be great to add at least a comment outlining the different valid states of this object for future readers. Presumably this should be derivable by looking at all the constructor overloads, but that's much more complex than a single comment block that stands alone.

ras0219-msft · 2024-05-01T20:03:48Z

include/vcpkg/base/diagnostics.h

+        // Prints this diagnostic to the terminal.
+        // Not thread safe: The console DiagnosticContext must apply its own synchronization.


I'm confused by this comment since MessageSink might not be a terminal.

Why is console DiagnosticContext mentioned in this comment, when the function takes a MessageSink?

ras0219-msft · 2024-05-01T20:25:15Z

include/vcpkg/base/message-data.inc.h

-                "https://github.com/microsoft/vcpkg, "
-                "with the following output:\n{error_msg}\nSerialized Binary Paragraph:")
+DECLARE_MESSAGE(
+    FailedToParseSerializedBinParagraphSuffix,


This message seems substantially more generic now than just failing to parse a serialized binary paragraph. Could this get a more descriptive name?

ras0219-msft · 2024-05-01T20:29:24Z

include/vcpkg/base/parse.h

-        ParserBase(StringView text, Optional<StringView> origin, TextRowCol init_rowcol = {});
+        // When parsing an in memory entity or similar not-on-disk entity, init_row should be set to 0
+        // When parsing a file, init_rowcol should be set to 1 if starting from the top of the file
+        ParserBase(DiagnosticContext& context, StringView text, Optional<StringView> origin, int init_row);


Is it valid to pass nullopt, 1? If not, then should init_row be moved into the Optional<> via a pair or such?

Suggested change

ParserBase(DiagnosticContext& context, StringView text, Optional<StringView> origin, int init_row);

ParserBase(DiagnosticContext& context, StringView text, Optional<std::pair<StringView, int>> origin);

Or, should this just be overloads?

Suggested change

ParserBase(DiagnosticContext& context, StringView text, Optional<StringView> origin, int init_row);

ParserBase(DiagnosticContext& context, StringView text);

// Triggers fatal error on origin.empty()

ParserBase(DiagnosticContext& context, StringView text, StringView origin, int init_row);

ras0219-msft · 2024-05-01T20:33:43Z

include/vcpkg/ci-baseline.h

@@ -53,7 +53,9 @@ namespace vcpkg
        bool operator()(const PackageSpec& spec) const;
    };

-    std::vector<CiBaselineLine> parse_ci_baseline(StringView text, StringView origin, ParseMessages& messages);
+    Optional<std::vector<CiBaselineLine>> parse_ci_baseline(DiagnosticContext& context,


Should this include base/diagnostics.h?

Suggested change

Optional<std::vector<CiBaselineLine>> parse_ci_baseline(DiagnosticContext& context,

Optional<std::vector<CiBaselineLine>> parse_ci_baseline(ParserBase parse);

ras0219-msft · 2024-05-01T20:35:52Z

include/vcpkg/paragraphparser.h

@@ -43,10 +43,18 @@ namespace vcpkg
        std::vector<LocalizedString> errors;
    };

+    Optional<std::vector<std::string>> parse_default_features_list(DiagnosticContext& context,


Suggested change

Optional<std::vector<std::string>> parse_default_features_list(DiagnosticContext& context,

Optional<std::vector<std::string>> parse_default_features_list(ParserBase& context);

BillyONeal added the depends:different-pr This PR depends on a different PR which has been filed label Jan 10, 2024

BillyONeal commented Jan 10, 2024

View reviewed changes

include/vcpkg/base/diagnostics.h Outdated Show resolved Hide resolved

BillyONeal commented Jan 10, 2024

View reviewed changes

include/vcpkg/base/diagnostics.h Outdated Show resolved Hide resolved

BillyONeal commented Jan 10, 2024

View reviewed changes

include/vcpkg/base/parse.h Outdated Show resolved Hide resolved

BillyONeal commented Jan 10, 2024

View reviewed changes

src/vcpkg-test/ci-baseline.cpp Outdated Show resolved Hide resolved

BillyONeal marked this pull request as draft January 10, 2024 19:59

BillyONeal removed the depends:different-pr This PR depends on a different PR which has been filed label Jan 26, 2024

BillyONeal added 11 commits January 26, 2024 13:47

Add color printing.

0223881

Add unique_ptr handling.

94722bb

Forward in more places.

ba58259

More tests and bugfixes.

bbd39f2

More testing!

1549d60

Tests, bugfixes, comments. Simplify the unique_ptr adapter to use ref…

164a8dc

…erence-collapsing.

First crack at applying DiagnosticContext to ParserBase.

fbcf1e7

Fix unit test failures.

9396f8a

Revert the TextRowCol rename

6e3f487

Minor self CR feedback.

9fdf0e2

BillyONeal force-pushed the error-document branch from ca6d48d to 9fdf0e2 Compare January 26, 2024 21:54

BillyONeal added 3 commits January 26, 2024 13:56

Remove unused WarningsTreatedAsErrors

04c46ec

Avoid -Wunneeded-internal-declaration

e0287e6

Avoid -Wmissing-prototypes

ddb26ce

BillyONeal marked this pull request as ready for review January 26, 2024 23:50

BillyONeal requested a review from ras0219-msft January 27, 2024 00:02

ras0219-msft reviewed Feb 14, 2024

View reviewed changes

BillyONeal added 2 commits February 21, 2024 19:34

clang-format

de35a73

BillyONeal force-pushed the error-document branch from f69db32 to b28aadc Compare April 20, 2024 02:16

BillyONeal added 5 commits April 22, 2024 14:32

Merge remote-tracking branch 'origin/main' into error-document

24ac5bf

Correctly track row information and avoid emitting diagnostics for sp…

f7c13ab

…eculative parsing. Note that TextRowCol was downgraded to int because ParserBase::ParserBase assumes start_of_line == the content.

Dedupe CLI e2e tests.

87a6c0e

Fix posix build

1616bbd

Downgrade clang-format to 16

3159407

autoantwort mentioned this pull request Apr 30, 2024

Make individual print calls thread safe #1290

Open

autoantwort reviewed Apr 30, 2024

View reviewed changes

Crzyrndm mentioned this pull request Jun 6, 2024

parallel operations for some binary caching providers #1392

Open

2 tasks

BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Jun 7, 2024

Revert the ReaderMessage part of the change which became microsoft#…

fa86593

…1323 as requested by @ras0219-msft

BillyONeal added 4 commits July 25, 2024 14:02

Merge remote-tracking branch 'origin/main' into error-document

bab1477

# Conflicts: # include/vcpkg/base/message-data.inc.h # locales/messages.json # src/vcpkg/base/json.cpp # src/vcpkg/commands.ci-verify-versions.cpp # src/vcpkg/commands.format-manifest.cpp # src/vcpkg/paragraphs.cpp

clang-format

159f9f3

clang-format again?

2224689

Rename print to print_to as suggested by @autoantwort

5cad35c

ras0219-msft approved these changes Nov 20, 2024

View reviewed changes

BillyONeal added 4 commits December 6, 2024 15:31

Merge remote-tracking branch 'origin/main' into error-document

2a2bcf5

Revert most everything related to ParserBase.

a817cf7

Format

75f997b

Fix test failures oops

c1239d3

BillyONeal enabled auto-merge (squash) December 7, 2024 00:09

format

bfed436

BillyONeal merged commit b4b372c into microsoft:main Dec 7, 2024
6 checks passed

BillyONeal deleted the error-document branch December 7, 2024 00:48

Thomas1664 mentioned this pull request Dec 8, 2024

Install files in parallel #1256

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce DiagnosticContext, an 'error document' like type. #1323

Introduce DiagnosticContext, an 'error document' like type. #1323

BillyONeal commented Jan 10, 2024 •

edited

Loading

BillyONeal commented Jan 27, 2024

autoantwort commented Jan 27, 2024

autoantwort commented Jan 27, 2024

BillyONeal commented Jan 29, 2024

ras0219-msft commented Feb 14, 2024 •

edited

Loading

ras0219-msft Feb 14, 2024

BillyONeal Feb 15, 2024

ras0219-msft Feb 14, 2024

BillyONeal Feb 15, 2024

BillyONeal Feb 15, 2024

ras0219-msft May 1, 2024 •

edited

Loading

ras0219-msft Feb 14, 2024

BillyONeal Feb 15, 2024

BillyONeal Apr 22, 2024

ras0219-msft May 1, 2024

ras0219-msft Feb 14, 2024

BillyONeal Apr 22, 2024

autoantwort commented Apr 20, 2024

BillyONeal commented Apr 22, 2024

Thomas1664 commented Apr 28, 2024

autoantwort left a comment

autoantwort Apr 30, 2024

autoantwort Apr 30, 2024

BillyONeal May 1, 2024

autoantwort Apr 30, 2024

BillyONeal May 1, 2024

BillyONeal commented May 1, 2024 •

edited

Loading

ras0219-msft May 1, 2024

ras0219-msft May 1, 2024

ras0219-msft May 1, 2024

ras0219-msft May 1, 2024

ras0219-msft May 1, 2024

ras0219-msft May 1, 2024

	void print(MessageSink& sink) const;
	void print_to(MessageSink& sink) const;

		// Prints this diagnostic to the terminal.
		// Not thread safe: The console DiagnosticContext must apply its own synchronization.

	ParserBase(DiagnosticContext& context, StringView text, Optional<StringView> origin, int init_row);
	ParserBase(DiagnosticContext& context, StringView text, Optional<std::pair<StringView, int>> origin);

-        ParserBase(DiagnosticContext& context, StringView text, Optional<StringView> origin, int init_row);
+        ParserBase(DiagnosticContext& context, StringView text);
+        // Triggers fatal error on origin.empty()
+        ParserBase(DiagnosticContext& context, StringView text, StringView origin, int init_row);

	Optional<std::vector<CiBaselineLine>> parse_ci_baseline(DiagnosticContext& context,
	Optional<std::vector<CiBaselineLine>> parse_ci_baseline(ParserBase parse);

	Optional<std::vector<std::string>> parse_default_features_list(DiagnosticContext& context,
	Optional<std::vector<std::string>> parse_default_features_list(ParserBase& context);

Introduce DiagnosticContext, an 'error document' like type. #1323

Introduce DiagnosticContext, an 'error document' like type. #1323

Conversation

BillyONeal commented Jan 10, 2024 • edited Loading

BillyONeal commented Jan 27, 2024

autoantwort commented Jan 27, 2024

autoantwort commented Jan 27, 2024

BillyONeal commented Jan 29, 2024

ras0219-msft commented Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ras0219-msft May 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

autoantwort commented Apr 20, 2024

BillyONeal commented Apr 22, 2024

Thomas1664 commented Apr 28, 2024

autoantwort left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BillyONeal commented May 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BillyONeal commented Jan 10, 2024 •

edited

Loading

ras0219-msft commented Feb 14, 2024 •

edited

Loading

ras0219-msft May 1, 2024 •

edited

Loading

BillyONeal commented May 1, 2024 •

edited

Loading