From e696aabd11beb80a2569eafe0d8c5eb48ae29c6b Mon Sep 17 00:00:00 2001 From: ToruNiina Date: Mon, 27 Jul 2020 00:48:04 +0900 Subject: [PATCH] refactor: change internal interface to reduce code to remove `std::addressof` calls, get_region(toml::value) now returns a pointer to region. --- toml/get.hpp | 20 ++++++++-------- toml/parser.hpp | 63 +++++++++++++++++++++---------------------------- toml/value.hpp | 31 +++++++++++------------- 3 files changed, 51 insertions(+), 63 deletions(-) diff --git a/toml/get.hpp b/toml/get.hpp index 93b834a..d2353a6 100644 --- a/toml/get.hpp +++ b/toml/get.hpp @@ -189,7 +189,7 @@ get(const basic_value& v) { throw type_error(detail::format_underline("toml::value: " "bad_cast to std::chrono::system_clock::time_point", { - {std::addressof(detail::get_region(v)), + {detail::get_region(v), concat_to_string("the actual type is ", v.type())} }), v.location()); } @@ -336,7 +336,7 @@ get(const basic_value& v) throw std::out_of_range(detail::format_underline(concat_to_string( "toml::get: specified container size is ", container.size(), " but there are ", ar.size(), " elements in toml array."), { - {std::addressof(detail::get_region(v)), "here"} + {detail::get_region(v), "here"} })); } std::transform(ar.cbegin(), ar.cend(), container.begin(), @@ -361,7 +361,7 @@ get(const basic_value& v) throw std::out_of_range(detail::format_underline(concat_to_string( "toml::get: specified std::pair but there are ", ar.size(), " elements in toml array."), { - {std::addressof(detail::get_region(v)), "here"} + {detail::get_region(v), "here"} })); } return std::make_pair(::toml::get(ar.at(0)), @@ -393,7 +393,7 @@ get(const basic_value& v) "toml::get: specified std::tuple with ", std::tuple_size::value, " elements, but there are ", ar.size(), " elements in toml array."), { - {std::addressof(detail::get_region(v)), "here"} + {detail::get_region(v), "here"} })); } return detail::get_tuple_impl(ar, @@ -512,7 +512,7 @@ find(const basic_value& v, const std::size_t idx) { throw std::out_of_range(detail::format_underline(concat_to_string( "index ", idx, " is out of range"), { - {std::addressof(detail::get_region(v)), "in this array"} + {detail::get_region(v), "in this array"} })); } return ary.at(idx); @@ -526,7 +526,7 @@ basic_value& find(basic_value& v, const std::size_t idx) { throw std::out_of_range(detail::format_underline(concat_to_string( "index ", idx, " is out of range"), { - {std::addressof(detail::get_region(v)), "in this array"} + {detail::get_region(v), "in this array"} })); } return ary.at(idx); @@ -540,7 +540,7 @@ basic_value find(basic_value&& v, const std::size_t idx) { throw std::out_of_range(detail::format_underline(concat_to_string( "index ", idx, " is out of range"), { - {std::addressof(detail::get_region(v)), "in this array"} + {detail::get_region(v), "in this array"} })); } return basic_value(std::move(ary.at(idx))); @@ -600,7 +600,7 @@ find(const basic_value& v, const std::size_t idx) { throw std::out_of_range(detail::format_underline(concat_to_string( "index ", idx, " is out of range"), { - {std::addressof(detail::get_region(v)), "in this array"} + {detail::get_region(v), "in this array"} })); } return ::toml::get(ary.at(idx)); @@ -615,7 +615,7 @@ find(basic_value& v, const std::size_t idx) { throw std::out_of_range(detail::format_underline(concat_to_string( "index ", idx, " is out of range"), { - {std::addressof(detail::get_region(v)), "in this array"} + {detail::get_region(v), "in this array"} })); } return ::toml::get(ary.at(idx)); @@ -630,7 +630,7 @@ find(basic_value&& v, const std::size_t idx) { throw std::out_of_range(detail::format_underline(concat_to_string( "index ", idx, " is out of range"), { - {std::addressof(detail::get_region(v)), "in this array"} + {detail::get_region(v), "in this array"} })); } return ::toml::get(std::move(ary.at(idx))); diff --git a/toml/parser.hpp b/toml/parser.hpp index 084bbd7..4e6d75f 100644 --- a/toml/parser.hpp +++ b/toml/parser.hpp @@ -986,11 +986,11 @@ parse_array(location& loc) "type of elements should be the same each other.", { {std::addressof(array_start_loc), "array starts here"}, { - std::addressof(get_region(retval.front())), + get_region(retval.front()), "value has type " + stringize(retval.front().type()) }, { - std::addressof(get_region(val.unwrap())), + get_region(val.unwrap()), "value has different type, " + stringize(val.unwrap().type()) } }), source_location(std::addressof(loc))); @@ -1161,7 +1161,7 @@ template bool is_valid_forward_table_definition(const Value& fwd, Iterator key_first, Iterator key_curr, Iterator key_last) { - location def("internal", detail::get_region(fwd).str()); + location def("internal", detail::get_region(fwd)->str()); if(const auto tabkeys = parse_table_key(def)) { // table keys always contains all the nodes from the root. @@ -1236,10 +1236,8 @@ insert_nested_key(typename Value::table_type& root, const Value& v, "toml::insert_value: array of table (\"", format_dotted_keys(first, last), "\") cannot be defined"), { - {std::addressof(get_region(tab->at(k))), - "table already defined"}, - {std::addressof(get_region(v)), - "this conflicts with the previous table"} + {get_region(tab->at(k)), "table already defined"}, + {get_region(v), "this conflicts with the previous table"} }), v.location()); } else if(!(tab->at(k).is_array())) @@ -1248,10 +1246,10 @@ insert_nested_key(typename Value::table_type& root, const Value& v, "toml::insert_value: array of table (\"", format_dotted_keys(first, last), "\") collides with" " existing value"), { - {std::addressof(get_region(tab->at(k))), + {get_region(tab->at(k)), concat_to_string("this ", tab->at(k).type(), " value already exists")}, - {std::addressof(get_region(v)), + {get_region(v), "while inserting this array-of-tables"} }), v.location()); } @@ -1263,10 +1261,10 @@ insert_nested_key(typename Value::table_type& root, const Value& v, "toml::insert_value: array of table (\"", format_dotted_keys(first, last), "\") collides with" " existing value"), { - {std::addressof(get_region(tab->at(k))), + {get_region(tab->at(k)), concat_to_string("this ", tab->at(k).type(), " value already exists")}, - {std::addressof(get_region(v)), + {get_region(v), "while inserting this array-of-tables"} }), v.location()); } @@ -1285,16 +1283,16 @@ insert_nested_key(typename Value::table_type& root, const Value& v, // that points to the key of the table (e.g. [[a]]). By // comparing the first two letters in key, we can detect // the array-of-table is inline or multiline. - if(detail::get_region(a.front()).str().substr(0,2) != "[[") + if(detail::get_region(a.front())->str().substr(0,2) != "[[") { throw syntax_error(format_underline(concat_to_string( "toml::insert_value: array of table (\"", format_dotted_keys(first, last), "\") collides with" " existing array-of-tables"), { - {std::addressof(get_region(tab->at(k))), + {get_region(tab->at(k)), concat_to_string("this ", tab->at(k).type(), " value has static size")}, - {std::addressof(get_region(v)), + {get_region(v), "appending it to the statically sized array"} }), v.location()); } @@ -1320,10 +1318,8 @@ insert_nested_key(typename Value::table_type& root, const Value& v, "toml::insert_value: table (\"", format_dotted_keys(first, last), "\") already exists."), { - {std::addressof(get_region(tab->at(k))), - "table already exists here"}, - {std::addressof(get_region(v)), - "table defined twice"} + {get_region(tab->at(k)), "table already exists here"}, + {get_region(v), "table defined twice"} }), v.location()); } // to allow the following toml file. @@ -1347,10 +1343,8 @@ insert_nested_key(typename Value::table_type& root, const Value& v, throw syntax_error(format_underline(concat_to_string( "toml::insert_value: array of tables (\"", format_dotted_keys(first, last), "\") already exists."), { - {std::addressof(get_region(tab->at(k))), - "array of tables defined here"}, - {std::addressof(get_region(v)), - "table conflicts with the previous array of table"} + {get_region(tab->at(k)), "array of tables defined here"}, + {get_region(v), "table conflicts with the previous array of table"} }), v.location()); } else @@ -1358,10 +1352,8 @@ insert_nested_key(typename Value::table_type& root, const Value& v, throw syntax_error(format_underline(concat_to_string( "toml::insert_value: value (\"", format_dotted_keys(first, last), "\") already exists."), { - {std::addressof(get_region(tab->at(k))), - "value already exists here"}, - {std::addressof(get_region(v)), - "value defined twice"} + {get_region(tab->at(k)), "value already exists here"}, + {get_region(v), "value defined twice"} }), v.location()); } } @@ -1390,15 +1382,14 @@ insert_nested_key(typename Value::table_type& root, const Value& v, { // here, if the value is a (multi-line) table, the region // should be something like `[table-name]`. - if(get_region(tab->at(k)).front() == '{') + if(get_region(tab->at(k))->front() == '{') { throw syntax_error(format_underline(concat_to_string( "toml::insert_value: inserting to an inline table (", format_dotted_keys(first, std::next(iter)), ") but inline tables are immutable"), { - {std::addressof(get_region(tab->at(k))), - "inline tables are immutable"}, - {std::addressof(get_region(v)), "inserting this"} + {get_region(tab->at(k)), "inline tables are immutable"}, + {get_region(v), "inserting this"} }), v.location()); } } @@ -1413,9 +1404,9 @@ insert_nested_key(typename Value::table_type& root, const Value& v, "toml::insert_value: target (", format_dotted_keys(first, std::next(iter)), ") is neither table nor an array of tables"), { - {std::addressof(get_region(a.back())), - concat_to_string("actual type is ", a.back().type())}, - {std::addressof(get_region(v)), "inserting this"} + {get_region(a.back()), concat_to_string( + "actual type is ", a.back().type())}, + {get_region(v), "inserting this"} }), v.location()); } tab = std::addressof(a.back().as_table()); @@ -1426,9 +1417,9 @@ insert_nested_key(typename Value::table_type& root, const Value& v, "toml::insert_value: target (", format_dotted_keys(first, std::next(iter)), ") is neither table nor an array of tables"), { - {std::addressof(get_region(tab->at(k))), - concat_to_string("actual type is ", tab->at(k).type())}, - {std::addressof(get_region(v)), "inserting this"} + {get_region(tab->at(k)), concat_to_string( + "actual type is ", tab->at(k).type())}, + {get_region(v), "inserting this"} }), v.location()); } } diff --git a/toml/value.hpp b/toml/value.hpp index d0af508..b74aac5 100644 --- a/toml/value.hpp +++ b/toml/value.hpp @@ -22,9 +22,9 @@ namespace detail // to show error messages. not recommended for users. template -inline region_base const& get_region(const Value& v) +inline region_base const* get_region(const Value& v) { - return *(v.region_info_); + return v.region_info_.get(); } template @@ -46,8 +46,7 @@ throw_bad_cast(const std::string& funcname, value_t actual, const Value& v) { throw type_error(detail::format_underline( concat_to_string(funcname, "bad_cast to ", Expected), { - {std::addressof(get_region(v)), - concat_to_string("the actual type is ", actual)} + {get_region(v), concat_to_string("the actual type is ", actual)} }), v.location()); } @@ -74,8 +73,8 @@ throw_key_not_found_error(const Value& v, const key& ky) // It actually points to the top-level table at the first character, // not `[table]`. But it is too confusing. To avoid the confusion, the error // message should explicitly say "key not found in the top-level table". - const auto& reg = get_region(v); - if(reg.line_num() == "1" && reg.size() == 1) + const auto* reg = get_region(v); + if(reg->line_num() == "1" && reg->size() == 1) { // Here it assumes that top-level table starts at the first character. // The region corresponds to the top-level table will be generated at @@ -111,16 +110,14 @@ throw_key_not_found_error(const Value& v, const key& ky) // throw std::out_of_range(format_underline(concat_to_string( "key \"", ky, "\" not found in the top-level table"), { - {std::addressof(reg), "the top-level table starts here"} + {reg, "the top-level table starts here"} })); } else { // normal table. throw std::out_of_range(format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(reg), "in this table"} - })); + "key \"", ky, "\" not found"), { {reg, "in this table"} })); } } @@ -1710,7 +1707,7 @@ class basic_value // for error messages template - friend region_base const& detail::get_region(const Value& v); + friend region_base const* detail::get_region(const Value& v); template friend void detail::change_region(Value& v, Region&& reg); @@ -1920,7 +1917,7 @@ inline std::string format_error(const std::string& err_msg, { return detail::format_underline(err_msg, std::vector>{ - {std::addressof(detail::get_region(v)), comment} + {detail::get_region(v), comment} }, std::move(hints), colorize); } @@ -1933,8 +1930,8 @@ inline std::string format_error(const std::string& err_msg, { return detail::format_underline(err_msg, std::vector>{ - {std::addressof(detail::get_region(v1)), comment1}, - {std::addressof(detail::get_region(v2)), comment2} + {detail::get_region(v1), comment1}, + {detail::get_region(v2), comment2} }, std::move(hints), colorize); } @@ -1948,9 +1945,9 @@ inline std::string format_error(const std::string& err_msg, { return detail::format_underline(err_msg, std::vector>{ - {std::addressof(detail::get_region(v1)), comment1}, - {std::addressof(detail::get_region(v2)), comment2}, - {std::addressof(detail::get_region(v3)), comment3} + {detail::get_region(v1), comment1}, + {detail::get_region(v2), comment2}, + {detail::get_region(v3), comment3} }, std::move(hints), colorize); }