From d1af42f151820aeb48dee02a78f3f1eb0167f2e0 Mon Sep 17 00:00:00 2001 From: ToruNiina Date: Sat, 29 Feb 2020 22:23:15 +0900 Subject: [PATCH 1/3] refactor: add throw_key_not_found_error and replace related throw statements with it --- toml/get.hpp | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/toml/get.hpp b/toml/get.hpp index 4a2f045..1d33650 100644 --- a/toml/get.hpp +++ b/toml/get.hpp @@ -9,6 +9,20 @@ namespace toml { +namespace detail +{ +// Throw from `toml::find`, generating an error message +template class M, template class V> +[[noreturn]] void +throw_key_not_found_error(const basic_value& v, const key& ky) +{ + throw std::out_of_range(format_underline(concat_to_string( + "key \"", ky, "\" not found"), { + {std::addressof(get_region(v)), "in this table"} + })); +} +} // detail // ============================================================================ // exact toml::* type @@ -449,10 +463,7 @@ basic_value const& find(const basic_value& v, const key& ky) const auto& tab = v.as_table(); if(tab.count(ky) == 0) { - throw std::out_of_range(detail::format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(detail::get_region(v)), "in this table"} - })); + detail::throw_key_not_found_error(v, ky); } return tab.at(ky); } @@ -463,10 +474,7 @@ basic_value& find(basic_value& v, const key& ky) auto& tab = v.as_table(); if(tab.count(ky) == 0) { - throw std::out_of_range(detail::format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(detail::get_region(v)), "in this table"} - })); + detail::throw_key_not_found_error(v, ky); } return tab.at(ky); } @@ -477,10 +485,7 @@ basic_value find(basic_value&& v, const key& ky) typename basic_value::table_type tab = std::move(v).as_table(); if(tab.count(ky) == 0) { - throw std::out_of_range(detail::format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(detail::get_region(v)), "in this table"} - })); + detail::throw_key_not_found_error(v, ky); } return basic_value(std::move(tab.at(ky))); } @@ -542,10 +547,7 @@ find(const basic_value& v, const key& ky) const auto& tab = v.as_table(); if(tab.count(ky) == 0) { - throw std::out_of_range(detail::format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(detail::get_region(v)), "in this table"} - })); + detail::throw_key_not_found_error(v, ky); } return ::toml::get(tab.at(ky)); } @@ -558,10 +560,7 @@ find(basic_value& v, const key& ky) auto& tab = v.as_table(); if(tab.count(ky) == 0) { - throw std::out_of_range(detail::format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(detail::get_region(v)), "in this table"} - })); + detail::throw_key_not_found_error(v, ky); } return ::toml::get(tab.at(ky)); } @@ -574,10 +573,7 @@ find(basic_value&& v, const key& ky) typename basic_value::table_type tab = std::move(v).as_table(); if(tab.count(ky) == 0) { - throw std::out_of_range(detail::format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(detail::get_region(v)), "in this table"} - })); + detail::throw_key_not_found_error(v, ky); } return ::toml::get(std::move(tab.at(ky))); } From 128b66bda97da4df20cb5c5d8be70ebaac412f8f Mon Sep 17 00:00:00 2001 From: ToruNiina Date: Sat, 29 Feb 2020 22:54:50 +0900 Subject: [PATCH 2/3] refactor: add missing whitespace --- toml/region.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toml/region.hpp b/toml/region.hpp index 77ba943..68defa5 100644 --- a/toml/region.hpp +++ b/toml/region.hpp @@ -55,7 +55,7 @@ struct region_base // number of characters in the line after the region virtual std::size_t after() const noexcept {return 0;} - virtual std::vector comments()const {return {};} + virtual std::vector comments() const {return {};} // ```toml // # comment_before // key = "value" # comment_inline From d11e42ca7ef1ece1871adba570b83cc8835f7658 Mon Sep 17 00:00:00 2001 From: ToruNiina Date: Sat, 29 Feb 2020 22:56:29 +0900 Subject: [PATCH 3/3] fix: explicitly say the table is top-level The top-level table has its region at the first character of the file. That means that, in the case when a key is not found in the top-level table, the error message points to the first character. If the file has its first table at the first line, the error message would be like this. ```console [error] key "a" not found --> example.toml | 1 | [table] | ^------ in this table ``` 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". --- toml/get.hpp | 71 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/toml/get.hpp b/toml/get.hpp index 1d33650..e4eb063 100644 --- a/toml/get.hpp +++ b/toml/get.hpp @@ -11,16 +11,77 @@ namespace toml { namespace detail { -// Throw from `toml::find`, generating an error message +// Throw out_of_range from `toml::find` after generating an error message. +// +// The implementation is a bit complicated and there are many edge-cases. +// If you are not interested in the error message generation, just skip this. template class M, template class V> [[noreturn]] void throw_key_not_found_error(const basic_value& v, const key& ky) { - throw std::out_of_range(format_underline(concat_to_string( - "key \"", ky, "\" not found"), { - {std::addressof(get_region(v)), "in this table"} - })); + // The top-level table has its region at the first character of the file. + // That means that, in the case when a key is not found in the top-level + // table, the error message points to the first character. If the file has + // its first table at the first line, the error message would be like this. + // ```console + // [error] key "a" not found + // --> example.toml + // | + // 1 | [table] + // | ^------ in this table + // ``` + // 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) + { + // Here it assumes that top-level table starts at the first character. + // The region corresponds to the top-level table will be generated at + // `parse_toml_file` function. + // It also assumes that the top-level table size is just one and + // the line number is `1`. It is always satisfied. And those conditions + // are satisfied only if the table is the top-level table. + // + // 1. one-character dot-key at the first line + // ```toml + // a.b = "c" + // ``` + // toml11 counts whole key as the table key. Here, `a.b` is the region + // of the table "a". It could be counter intuitive, but it works. + // The size of the region is 3, not 1. The above example is the shortest + // dot-key example. The size cannot be 1. + // + // 2. one-character inline-table at the first line + // ```toml + // a = {b = "c"} + // ``` + // toml11 consideres the inline table body as the table region. Here, + // `{b = "c"}` is the region of the table "a". The size of the region + // is 9, not 1. The shotest inline table still has two characters, `{` + // and `}`. The size cannot be 1. + // + // 3. one-character table declaration at the first line + // ```toml + // [a] + // ``` + // toml11 consideres the whole table key as the table region. Here, + // `[a]` is the table region. The size is 3, not 1. + // + 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"} + })); + } + else + { + // normal table. + throw std::out_of_range(format_underline(concat_to_string( + "key \"", ky, "\" not found"), { + {std::addressof(reg), "in this table"} + })); + } } } // detail