diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 52a8441..dfcf41f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -58,6 +58,8 @@ jobs: - {compiler: '3.9', standard: '20'} - {compiler: '4.0', standard: '20'} - {compiler: '5.0', standard: '20'} + - {compiler: '6.0', standard: '20'} + - {compiler: '7', standard: '20'} steps: - name: Checkout uses: actions/checkout@v2 @@ -76,7 +78,7 @@ jobs: - name: Configure run: | mkdir build && cd build - cmake .. -Dtoml11_BUILD_TEST=ON -DCMAKE_CXX_COMPILER=clang++-${{ matrix.compiler }} -DCMAKE_CXX_STANDARD=${{ matrix.standard }} + cmake .. -Dtoml11_BUILD_TEST=ON -DCMAKE_C_COMPILER=clang-${{ matrix.compiler }} -DCMAKE_CXX_COMPILER=clang++-${{ matrix.compiler }} -DCMAKE_CXX_STANDARD=${{ matrix.standard }} - name: Build run: | cd build && cmake --build . diff --git a/tests/test_comments.cpp b/tests/test_comments.cpp index 5ae1e0d..7438cae 100644 --- a/tests/test_comments.cpp +++ b/tests/test_comments.cpp @@ -138,6 +138,43 @@ BOOST_AUTO_TEST_CASE(test_comment_both) } } +BOOST_AUTO_TEST_CASE(test_comments_on_implicit_values) +{ + { + const std::string file = R"( + # comment for the first element of array-of-tables. + [[array-of-tables]] + foo = "bar" + )"; + std::istringstream iss(file); + const auto v = toml::parse(iss); + + const auto aot = toml::find(v, "array-of-tables"); + const auto elm = aot.at(0); + BOOST_TEST(aot.comments().empty()); + BOOST_TEST(elm.comments().size() == 1); + BOOST_TEST(elm.comments().front() == " comment for the first element of array-of-tables."); + } + { + const std::string file = R"( + # comment for the array itself + array-of-tables = [ + # comment for the first element of array-of-tables. + {foo = "bar"} + ] + )"; + std::istringstream iss(file); + const auto v = toml::parse(iss); + + const auto aot = toml::find(v, "array-of-tables"); + const auto elm = aot.at(0); + BOOST_TEST(aot.comments().size() == 1); + BOOST_TEST(aot.comments().front() == " comment for the array itself"); + BOOST_TEST(elm.comments().size() == 1); + BOOST_TEST(elm.comments().front() == " comment for the first element of array-of-tables."); + } +} + BOOST_AUTO_TEST_CASE(test_discard_comment) { const std::string file = R"( diff --git a/toml/parser.hpp b/toml/parser.hpp index a52d8c9..f6ba78a 100644 --- a/toml/parser.hpp +++ b/toml/parser.hpp @@ -1315,7 +1315,41 @@ insert_nested_key(typename Value::table_type& root, const Value& v, } else // if not, we need to create the array of table { - value_type aot(array_type(1, v), key_reg); + // XXX: Consider the following array of tables. + // ```toml + // # This is a comment. + // [[aot]] + // foo = "bar" + // ``` + // Here, the comment is for `aot`. But here, actually two + // values are defined. An array that contains tables, named + // `aot`, and the 0th element of the `aot`, `{foo = "bar"}`. + // Those two are different from each other. But both of them + // points to the same portion of the TOML file, `[[aot]]`, + // so `key_reg.comments()` returns `# This is a comment`. + // If it is assigned as a comment of `aot` defined here, the + // comment will be duplicated. Both the `aot` itself and + // the 0-th element will have the same comment. This causes + // "duplication of the same comments" bug when the data is + // serialized. + // Next, consider the following. + // ```toml + // # comment 1 + // aot = [ + // # coment 2 + // {foo = "bar"}, + // ] + // ``` + // In this case, we can distinguish those two comments. So + // here we need to add "comment 1" to the `aot` and + // "comment 2" to the 0th element of that. + // To distinguish those two, we check the key region. + std::vector comments{/* empty by default */}; + if(key_reg.str().substr(0, 2) != "[[") + { + comments = key_reg.comments(); + } + value_type aot(array_type(1, v), key_reg, std::move(comments)); tab->insert(std::make_pair(k, aot)); return ok(true); } @@ -1384,7 +1418,8 @@ insert_nested_key(typename Value::table_type& root, const Value& v, // [x.y.z] if(tab->count(k) == 0) { - (*tab)[k] = value_type(table_type{}, key_reg); + // a table that is defined implicitly doesn't have any comments. + (*tab)[k] = value_type(table_type{}, key_reg, {/*no comment*/}); } // type checking... @@ -1674,11 +1709,24 @@ inline result guess_value_type(const location& loc) } } +template +result +parse_value_helper(result, std::string> rslt) +{ + if(rslt.is_ok()) + { + auto comments = rslt.as_ok().second.comments(); + return ok(Value(std::move(rslt.as_ok()), std::move(comments))); + } + else + { + return err(std::move(rslt.as_err())); + } +} + template result parse_value(location& loc) { - using value_type = Value; - const auto first = loc.iter(); if(first == loc.end()) { @@ -1691,18 +1739,19 @@ result parse_value(location& loc) { return err(type.unwrap_err()); } + switch(type.unwrap()) { - case value_t::boolean : {return parse_boolean(loc); } - case value_t::integer : {return parse_integer(loc); } - case value_t::floating : {return parse_floating(loc); } - case value_t::string : {return parse_string(loc); } - case value_t::offset_datetime: {return parse_offset_datetime(loc);} - case value_t::local_datetime : {return parse_local_datetime(loc); } - case value_t::local_date : {return parse_local_date(loc); } - case value_t::local_time : {return parse_local_time(loc); } - case value_t::array : {return parse_array(loc); } - case value_t::table : {return parse_inline_table(loc);} + case value_t::boolean : {return parse_value_helper(parse_boolean(loc) );} + case value_t::integer : {return parse_value_helper(parse_integer(loc) );} + case value_t::floating : {return parse_value_helper(parse_floating(loc) );} + case value_t::string : {return parse_value_helper(parse_string(loc) );} + case value_t::offset_datetime: {return parse_value_helper(parse_offset_datetime(loc) );} + case value_t::local_datetime : {return parse_value_helper(parse_local_datetime(loc) );} + case value_t::local_date : {return parse_value_helper(parse_local_date(loc) );} + case value_t::local_time : {return parse_value_helper(parse_local_time(loc) );} + case value_t::array : {return parse_value_helper(parse_array(loc) );} + case value_t::table : {return parse_value_helper(parse_inline_table(loc));} default: { const auto msg = format_underline("toml::parse_value: " @@ -1993,7 +2042,7 @@ result parse_toml_file(location& loc) const auto& reg = tk.second; const auto inserted = insert_nested_key(data, - value_type(tab.unwrap(), reg), + value_type(tab.unwrap(), reg, reg.comments()), keys.begin(), keys.end(), reg, /*is_array_of_table=*/ true); if(!inserted) {return err(inserted.unwrap_err());} @@ -2010,7 +2059,8 @@ result parse_toml_file(location& loc) const auto& reg = tk.second; const auto inserted = insert_nested_key(data, - value_type(tab.unwrap(), reg), keys.begin(), keys.end(), reg); + value_type(tab.unwrap(), reg, reg.comments()), + keys.begin(), keys.end(), reg); if(!inserted) {return err(inserted.unwrap_err());} continue; @@ -2019,10 +2069,7 @@ result parse_toml_file(location& loc) "unknown line appeared", {{source_location(loc), "unknown format"}})); } - Value v(std::move(data), file); - v.comments() = comments; - - return ok(std::move(v)); + return ok(Value(std::move(data), file, comments)); } } // detail diff --git a/toml/serializer.hpp b/toml/serializer.hpp index 77cc71e..ed07f46 100644 --- a/toml/serializer.hpp +++ b/toml/serializer.hpp @@ -651,7 +651,7 @@ struct serializer tmp += '\n'; } - if(!kv.second.comments().empty() && !no_comment_ && tmp.front() != '#') + if(!kv.second.comments().empty() && !no_comment_) { for(const auto& c : kv.second.comments()) { diff --git a/toml/value.hpp b/toml/value.hpp index 76eee0d..e374c3b 100644 --- a/toml/value.hpp +++ b/toml/value.hpp @@ -1047,10 +1047,10 @@ class basic_value // // Those constructors take detail::region that contains parse result. - basic_value(boolean b, detail::region reg) + basic_value(boolean b, detail::region reg, std::vector cm) : type_(value_t::boolean), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->boolean_, b); } @@ -1058,68 +1058,75 @@ class basic_value detail::conjunction< std::is_integral, detail::negation> >::value, std::nullptr_t>::type = nullptr> - basic_value(T i, detail::region reg) + basic_value(T i, detail::region reg, std::vector cm) : type_(value_t::integer), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->integer_, static_cast(i)); } template::value, std::nullptr_t>::type = nullptr> - basic_value(T f, detail::region reg) + basic_value(T f, detail::region reg, std::vector cm) : type_(value_t::floating), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->floating_, static_cast(f)); } - basic_value(toml::string s, detail::region reg) + basic_value(toml::string s, detail::region reg, + std::vector cm) : type_(value_t::string), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->string_, std::move(s)); } - basic_value(const local_date& ld, detail::region reg) + basic_value(const local_date& ld, detail::region reg, + std::vector cm) : type_(value_t::local_date), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->local_date_, ld); } - basic_value(const local_time& lt, detail::region reg) + basic_value(const local_time& lt, detail::region reg, + std::vector cm) : type_(value_t::local_time), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->local_time_, lt); } - basic_value(const local_datetime& ldt, detail::region reg) + basic_value(const local_datetime& ldt, detail::region reg, + std::vector cm) : type_(value_t::local_datetime), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->local_datetime_, ldt); } - basic_value(const offset_datetime& odt, detail::region reg) + basic_value(const offset_datetime& odt, detail::region reg, + std::vector cm) : type_(value_t::offset_datetime), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->offset_datetime_, odt); } - basic_value(const array_type& ary, detail::region reg) + basic_value(const array_type& ary, detail::region reg, + std::vector cm) : type_(value_t::array), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->array_, ary); } - basic_value(const table_type& tab, detail::region reg) + basic_value(const table_type& tab, detail::region reg, + std::vector cm) : type_(value_t::table), region_info_(std::make_shared(std::move(reg))), - comments_(region_info_->comments()) + comments_(std::move(cm)) { assigner(this->table_, tab); } @@ -1127,8 +1134,10 @@ class basic_value template::value, std::nullptr_t>::type = nullptr> - basic_value(std::pair parse_result) - : basic_value(std::move(parse_result.first), std::move(parse_result.second)) + basic_value(std::pair parse_result, std::vector comments) + : basic_value(std::move(parse_result.first), + std::move(parse_result.second), + std::move(comments)) {} // type checking and casting ============================================