Add a new option to allow partial test output for non-Ok() messages. Adds a new `WithAllowPartialOutput()` method to `TextOutputOptions` to allow `WriteToString()` to write partial output when called with a non-`Ok()` view.
diff --git a/compiler/back_end/cpp/generated_code_templates b/compiler/back_end/cpp/generated_code_templates index 804544d..83a369d 100644 --- a/compiler/back_end/cpp/generated_code_templates +++ b/compiler/back_end/cpp/generated_code_templates
@@ -225,6 +225,8 @@ } } + static constexpr bool IsAggregate() { return true; } + $_field_method_declarations_$ private: @@ -305,40 +307,61 @@ continue; } - // ** write_field_to_text_stream ** //////////////////////////////////////////// if (has_$_field_name_$().ValueOr(false)) { - if (emboss_reserved_local_field_options.multiline()) { - emboss_reserved_local_stream->Write( - emboss_reserved_local_field_options.current_indent()); - } else { - if (emboss_reserved_local_wrote_field) { - emboss_reserved_local_stream->Write(","); + // Don't try to read the field if `allow_partial_output` is set and the + // field can't be `Read()`. Aggregates should still be visited, even if + // they are not `Ok()` overall, since submembers may still be `Ok()`. + if (!emboss_reserved_local_field_options.allow_partial_output() || + $_field_name_$().IsAggregate() || $_field_name_$().Ok()) { + if (emboss_reserved_local_field_options.multiline()) { + emboss_reserved_local_stream->Write( + emboss_reserved_local_field_options.current_indent()); + } else { + if (emboss_reserved_local_wrote_field) { + emboss_reserved_local_stream->Write(","); + } + emboss_reserved_local_stream->Write(" "); } - emboss_reserved_local_stream->Write(" "); - } - emboss_reserved_local_stream->Write("$_field_name_$: "); - $_field_name_$().WriteToTextStream(emboss_reserved_local_stream, - emboss_reserved_local_field_options); - emboss_reserved_local_wrote_field = true; - if (emboss_reserved_local_field_options.multiline()) { - emboss_reserved_local_stream->Write("\n"); + emboss_reserved_local_stream->Write("$_field_name_$: "); + $_field_name_$().WriteToTextStream(emboss_reserved_local_stream, + emboss_reserved_local_field_options); + emboss_reserved_local_wrote_field = true; + if (emboss_reserved_local_field_options.multiline()) { + emboss_reserved_local_stream->Write("\n"); + } + } else if (emboss_reserved_local_field_options.allow_partial_output() && + emboss_reserved_local_field_options.comments() && + !$_field_name_$().IsAggregate() && !$_field_name_$().Ok()) { + if (emboss_reserved_local_field_options.multiline()) { + emboss_reserved_local_stream->Write( + emboss_reserved_local_field_options.current_indent()); + } + emboss_reserved_local_stream->Write("# $_field_name_$: UNREADABLE\n"); } } - // ** write_read_only_field_to_text_stream ** ////////////////////////////////// if (has_$_field_name_$().ValueOr(false) && emboss_reserved_local_field_options.comments()) { - emboss_reserved_local_stream->Write( - emboss_reserved_local_field_options.current_indent()); - // TODO(bolms): When there are multiline read-only fields, add an option - // to TextOutputOptions to add `# ` to the current indent and use it here, - // so that subsequent lines are also commented out. - emboss_reserved_local_stream->Write("# $_field_name_$: "); - $_field_name_$().WriteToTextStream(emboss_reserved_local_stream, - emboss_reserved_local_field_options); - emboss_reserved_local_stream->Write("\n"); + if (!emboss_reserved_local_field_options.allow_partial_output() || + $_field_name_$().IsAggregate() || $_field_name_$().Ok()) { + emboss_reserved_local_stream->Write( + emboss_reserved_local_field_options.current_indent()); + // TODO(bolms): When there are multiline read-only fields, add an option + // to TextOutputOptions to add `# ` to the current indent and use it + // here, so that subsequent lines are also commented out. + emboss_reserved_local_stream->Write("# $_field_name_$: "); + $_field_name_$().WriteToTextStream(emboss_reserved_local_stream, + emboss_reserved_local_field_options); + emboss_reserved_local_stream->Write("\n"); + } else { + if (emboss_reserved_local_field_options.multiline()) { + emboss_reserved_local_stream->Write( + emboss_reserved_local_field_options.current_indent()); + } + emboss_reserved_local_stream->Write("# $_field_name_$: UNREADABLE\n"); + } } // ** constant_structure_size_method ** //////////////////////////////////////// @@ -520,6 +543,8 @@ ::emboss::support::$_write_to_text_stream_function_$( this, emboss_reserved_local_stream, emboss_reserved_local_options); } + + static constexpr bool IsAggregate() { return false; } }; static constexpr $_virtual_view_type_name_$ $_name_$() { @@ -622,6 +647,8 @@ this, emboss_reserved_local_stream, emboss_reserved_local_options); } + static constexpr bool IsAggregate() { return false; } + $_write_methods_$ private:
diff --git a/compiler/back_end/cpp/testcode/condition_test.cc b/compiler/back_end/cpp/testcode/condition_test.cc index 029abae..588a6f7 100644 --- a/compiler/back_end/cpp/testcode/condition_test.cc +++ b/compiler/back_end/cpp/testcode/condition_test.cc
@@ -759,6 +759,44 @@ EXPECT_EQ("{ x: 1 }", ::emboss::WriteToString(reader)); } +TEST(WriteToString, NotOkFieldsAreNotWritten) { + ::std::uint8_t buffer[2] = {0x00, 0x00}; + auto reader = BasicConditionalWriter(buffer, 1U); + EXPECT_FALSE(reader.Ok()); + EXPECT_EQ( + "{\n" + " x: 0 # 0x0\n" + " # xc: UNREADABLE\n" + "}", + ::emboss::WriteToString( + reader, ::emboss::MultilineText().WithAllowPartialOutput(true))); + EXPECT_EQ( + "{ x: 0 }", + ::emboss::WriteToString( + reader, ::emboss::TextOutputOptions().WithAllowPartialOutput(true))); +} + +TEST(WriteToString, NotOkStructSubFieldsAreNotWritten) { + ::std::uint8_t buffer[2] = {0x00, 0x00}; + auto reader = ConditionalInlineWriter(buffer, 2U); + EXPECT_FALSE(reader.Ok()); + EXPECT_EQ( + "{\n" + " payload_id: 0 # 0x0\n" + " type_0: {\n" + " a: 0 # 0x0\n" + " # b: UNREADABLE\n" + " # c: UNREADABLE\n" + " }\n" + "}", + ::emboss::WriteToString( + reader, ::emboss::MultilineText().WithAllowPartialOutput(true))); + EXPECT_EQ( + "{ payload_id: 0, type_0: { a: 0 } }", + ::emboss::WriteToString( + reader, ::emboss::TextOutputOptions().WithAllowPartialOutput(true))); +} + TEST(WriteToString, PresentFieldsNotWritten) { ::std::uint8_t buffer[2] = {0x00, 0x01}; auto reader = BasicConditionalWriter(buffer, 2U);
diff --git a/compiler/back_end/cpp/testcode/requires_test.cc b/compiler/back_end/cpp/testcode/requires_test.cc index 661fe6a..6a21efc 100644 --- a/compiler/back_end/cpp/testcode/requires_test.cc +++ b/compiler/back_end/cpp/testcode/requires_test.cc
@@ -201,6 +201,56 @@ EXPECT_FALSE(view.b_true().CouldWriteValue(false)); } +TEST(WriteToString, NotOkFieldsAreNotWritten) { + ::std::array</**/ ::std::uint8_t, 3> buffer = {0x00, 0x00, 0x00}; + auto reader = MakeRequiresIntegersView(&buffer); + EXPECT_FALSE(reader.Ok()); + EXPECT_EQ( + "{\n" + " zero_through_nine: 0 # 0x0\n" + " # ten_through_twenty: UNREADABLE\n" + " disjoint: 0 # 0x0\n" + " # ztn_plus_ttt: UNREADABLE\n" + " # alias_of_zero_through_nine: UNREADABLE\n" + " zero_through_nine_plus_five: 5 # 0x5\n" + "}", + ::emboss::WriteToString( + reader, ::emboss::MultilineText().WithAllowPartialOutput(true))); + EXPECT_EQ( + "{ zero_through_nine: 0, disjoint: 0, zero_through_nine_plus_five: 5 }", + ::emboss::WriteToString( + reader, ::emboss::TextOutputOptions().WithAllowPartialOutput(true))); +} + +TEST(WriteToString, NotOkArrayElementsAreNotWritten) { + ::std::array</**/ ::std::uint8_t, 4> buffer = {0x20, 0x00, 0x30, 0x05}; + auto reader = MakeRequiresInArrayElementsView(&buffer); + EXPECT_FALSE(reader.Ok()); + EXPECT_EQ( + "{\n" + " xs: {\n" + " [0]: {\n" + " # x: UNREADABLE\n" + " }\n" + " [1]: {\n" + " x: 0 # 0x0\n" + " }\n" + " [2]: {\n" + " # x: UNREADABLE\n" + " }\n" + " [3]: {\n" + " x: 5 # 0x5\n" + " }\n" + " }\n" + "}", + ::emboss::WriteToString( + reader, ::emboss::MultilineText().WithAllowPartialOutput(true))); + EXPECT_EQ( + "{ xs: { [0]: { }, { x: 0 }, { }, { x: 5 } } }", + ::emboss::WriteToString( + reader, ::emboss::TextOutputOptions().WithAllowPartialOutput(true))); +} + } // namespace } // namespace test } // namespace emboss
diff --git a/doc/cpp-reference.md b/doc/cpp-reference.md index 46ecdd0..779d777 100644 --- a/doc/cpp-reference.md +++ b/doc/cpp-reference.md
@@ -1902,7 +1902,9 @@ `PlusOneIndent` returns a new `TextOutputOptions` with one more level of indentation than the current `TextOutputOptions`. This is primarily intended for use inside of `WriteToTextStream` methods, as a way to get an indented -`TextOutputOptions` to pass to the `WriteToTextStream` methods of child objects. +`TextOutputOptions` to pass to the `WriteToTextStream` methods of child +objects. However, application callers may use `PlusOneIndent()`, possibly +multiple times, to indent the entire output. ### `Multiline` method @@ -1950,10 +1952,19 @@ `TextOutputOptions`, except for a new value for `digit_grouping()`. The new numeric base should be 2, 10, or 16. +### `WithAllowPartialOutput` method + +```c++ +TextOutputOptions WithAllowPartialOutput(bool new_value) const; +``` + +Returns a new `TextOutputOptions` with the same options as the current +`TextOutputOptions`, except for a new value for `allow_partial_output()`. + ### `current_indent` method ```c++ -::std::string current_indent() const; +::std::string current_indent() const; // Default "". ``` Returns the current indent string. @@ -1961,16 +1972,16 @@ ### `indent` method ```c++ -::std::string indent() const; +::std::string indent() const; // Default " ". ``` -Returns the indent string. The indent string is the string used for a single -level of indentation; most callers will prefer `current_indent`. +Returns the indent string. The indent string is the string used for a *single* +level of indentation. ### `multiline` method ```c++ -bool multiline() const; +bool multiline() const; // Default false. ``` Returns `true` if text output should use multiple lines, or `false` if text @@ -1979,7 +1990,7 @@ ### `digit_grouping` method ```c++ -bool digit_grouping() const; +bool digit_grouping() const; // Default false. ``` Returns `true` if text output should include digit separators on numbers; i.e. @@ -1988,7 +1999,7 @@ ### `comments` method ```c++ -bool comments() const; +bool comments() const; // Default false. ``` Returns `true` if text output should include comments, e.g., to show numbers in @@ -1997,8 +2008,25 @@ ### `numeric_base` method ```c++ -uint8_t numeric_base() const; +uint8_t numeric_base() const; // Default 10. ``` Returns the numeric base that should be used for formatting numbers. This should always be 2, 10, or 16. + +### `allow_partial_output` method + +```c++ +bool allow_partial_output() const; // Default false. +``` + +Returns `true` if text output should attempt to extract fields from a view that +is not `Ok()`. If so: + +* `WriteToString()` or `WriteToTextStream()` should never `CHECK`-fail. +* Atomic fields (e.g., `Int`, `UInt`, `enum`, `Flag`, etc. types) will not be + written to the text stream if they cannot be read. +* If `comments()` is also `true`, unreadable atomic fields will be commented + in the text stream. +* Aggregate fields (`struct`, `bits`, or arrays) will be written, but may be + missing fields or entirely empty if they have non-`Ok()` members.
diff --git a/runtime/cpp/emboss_array_view.h b/runtime/cpp/emboss_array_view.h index e222436..a1544c6 100644 --- a/runtime/cpp/emboss_array_view.h +++ b/runtime/cpp/emboss_array_view.h
@@ -248,6 +248,8 @@ WriteArrayToTextStream(this, stream, options); } + static constexpr bool IsAggregate() { return true; } + BufferType BackingStorage() const { return buffer_; } // Forwards to BufferType's ToString(), if any, but only if ElementView is a
diff --git a/runtime/cpp/emboss_enum_view.h b/runtime/cpp/emboss_enum_view.h index f59cd6d..af4d7aa 100644 --- a/runtime/cpp/emboss_enum_view.h +++ b/runtime/cpp/emboss_enum_view.h
@@ -151,6 +151,8 @@ ::emboss::support::WriteEnumViewToTextStream(this, stream, options); } + static constexpr bool IsAggregate() { return false; } + static constexpr int SizeInBits() { return Parameters::kBits; } private:
diff --git a/runtime/cpp/emboss_prelude.h b/runtime/cpp/emboss_prelude.h index 263b802..463f0fa 100644 --- a/runtime/cpp/emboss_prelude.h +++ b/runtime/cpp/emboss_prelude.h
@@ -118,6 +118,8 @@ ::emboss::support::WriteBooleanViewToTextStream(this, stream, options); } + static constexpr bool IsAggregate() { return false; } + private: BitBlock bit_block_; }; @@ -297,6 +299,8 @@ support::WriteIntegerViewToTextStream(this, stream, options); } + static constexpr bool IsAggregate() { return false; } + static constexpr int SizeInBits() { return Parameters::kBits; } private: @@ -460,6 +464,8 @@ support::WriteIntegerViewToTextStream(this, stream, options); } + static constexpr bool IsAggregate() { return false; } + static constexpr int SizeInBits() { return Parameters::kBits; } private: @@ -649,6 +655,8 @@ support::WriteIntegerViewToTextStream(this, stream, options); } + static constexpr bool IsAggregate() { return false; } + template <typename OtherView> void CopyFrom(const OtherView &other) const { Write(other.Read()); @@ -789,6 +797,8 @@ support::WriteFloatToTextStream(Read(), stream, options); } + static constexpr bool IsAggregate() { return false; } + static constexpr int SizeInBits() { return Parameters::kBits; } private:
diff --git a/runtime/cpp/emboss_text_util.h b/runtime/cpp/emboss_text_util.h index 8640851..59bdc4e 100644 --- a/runtime/cpp/emboss_text_util.h +++ b/runtime/cpp/emboss_text_util.h
@@ -73,12 +73,19 @@ return result; } + TextOutputOptions WithAllowPartialOutput(bool new_value) const { + TextOutputOptions result = *this; + result.allow_partial_output_ = new_value; + return result; + } + ::std::string current_indent() const { return current_indent_; } ::std::string indent() const { return indent_; } bool multiline() const { return multiline_; } bool digit_grouping() const { return digit_grouping_; } bool comments() const { return comments_; } ::std::uint8_t numeric_base() const { return numeric_base_; } + bool allow_partial_output() const { return allow_partial_output_; } private: ::std::string indent_; @@ -86,6 +93,7 @@ bool comments_ = false; bool multiline_ = false; bool digit_grouping_ = false; + bool allow_partial_output_ = false; ::std::uint8_t numeric_base_ = 10; }; @@ -691,36 +699,63 @@ stream->Write("{"); WriteShorthandArrayCommentToTextStream(array, stream, element_options); for (::std::size_t i = 0; i < array->ElementCount(); ++i) { - stream->Write("\n"); - stream->Write(element_options.current_indent()); - stream->Write("["); - // TODO(bolms): Put padding in here so that array elements start at the - // same column. - // - // TODO(bolms): (Maybe) figure out how to get padding to work so that - // elements with comments can have their comments align to the same - // column. - WriteIntegerToTextStream(i, stream, options.numeric_base(), - options.digit_grouping()); - stream->Write("]: "); - (*array)[i].WriteToTextStream(stream, element_options); + if (!options.allow_partial_output() || (*array)[i].IsAggregate() || + (*array)[i].Ok()) { + stream->Write("\n"); + stream->Write(element_options.current_indent()); + stream->Write("["); + // TODO(bolms): Put padding in here so that array elements start at the + // same column. + // + // TODO(bolms): (Maybe) figure out how to get padding to work so that + // elements with comments can have their comments align to the same + // column. + WriteIntegerToTextStream(i, stream, options.numeric_base(), + options.digit_grouping()); + stream->Write("]: "); + (*array)[i].WriteToTextStream(stream, element_options); + } else if (element_options.comments()) { + stream->Write("\n"); + stream->Write(element_options.current_indent()); + stream->Write("# ["); + WriteIntegerToTextStream(i, stream, options.numeric_base(), + options.digit_grouping()); + stream->Write("]: UNREADABLE"); + } } stream->Write("\n"); stream->Write(options.current_indent()); stream->Write("}"); } else { stream->Write("{"); + bool skipped_unreadable = false; for (::std::size_t i = 0; i < array->ElementCount(); ++i) { - stream->Write(" "); - if (i % 8 == 0) { - stream->Write("["); - WriteIntegerToTextStream(i, stream, options.numeric_base(), - options.digit_grouping()); - stream->Write("]: "); - } - (*array)[i].WriteToTextStream(stream, element_options); - if (i < array->ElementCount() - 1) { - stream->Write(","); + if (!options.allow_partial_output() || (*array)[i].IsAggregate() || + (*array)[i].Ok()) { + stream->Write(" "); + if (i % 8 == 0 || skipped_unreadable) { + stream->Write("["); + WriteIntegerToTextStream(i, stream, options.numeric_base(), + options.digit_grouping()); + stream->Write("]: "); + } + (*array)[i].WriteToTextStream(stream, element_options); + if (i < array->ElementCount() - 1) { + stream->Write(","); + } + skipped_unreadable = false; + } else { + if (element_options.comments()) { + stream->Write(" # "); + if (i % 8 == 0) { + stream->Write("["); + WriteIntegerToTextStream(i, stream, options.numeric_base(), + options.digit_grouping()); + stream->Write("]: "); + } + stream->Write("UNREADABLE\n"); + } + skipped_unreadable = true; } } stream->Write(" }");
diff --git a/runtime/cpp/test/emboss_text_util_test.cc b/runtime/cpp/test/emboss_text_util_test.cc index b7af4b8..7541576 100644 --- a/runtime/cpp/test/emboss_text_util_test.cc +++ b/runtime/cpp/test/emboss_text_util_test.cc
@@ -420,6 +420,13 @@ EXPECT_EQ(2, new_options.numeric_base()); } +TEST(TextOutputOptions, WithAllowPartialOutput) { + TextOutputOptions options; + TextOutputOptions new_options = options.WithAllowPartialOutput(true); + EXPECT_FALSE(options.allow_partial_output()); + EXPECT_TRUE(new_options.allow_partial_output()); +} + // Small helper function for the various WriteIntegerToTextStream tests; just // sets up a stream, forwards its arguments to WriteIntegerToTextStream, and // then returns the text from the stream.
diff --git a/testdata/requires.emb b/testdata/requires.emb index b37f61a..3317a98 100644 --- a/testdata/requires.emb +++ b/testdata/requires.emb
@@ -85,3 +85,10 @@ if b_exists: 2 [+1] Flag b_true [requires: this] + + +struct RequiresInArrayElements: + struct Element: + 0 [+1] UInt:8 x [requires: 0 <= this <= 10] + + 0 [+4] Element[] xs