diff --git a/opm/common/utility/OpmInputError.hpp b/opm/common/utility/OpmInputError.hpp index 410256ce0..13a501705 100644 --- a/opm/common/utility/OpmInputError.hpp +++ b/opm/common/utility/OpmInputError.hpp @@ -66,10 +66,15 @@ public: OpmInputError("Error at line {line} in file {file} - keyword: {keyword} has invalid argument {}", invalid_argument); */ - OpmInputError(const std::string& msg_fmt, const KeywordLocation& loc) : - m_what { OpmInputError::format(msg_fmt, loc) }, - locations { loc } - {} + template + OpmInputError(const std::string& reason, const KeywordLocation& location, const Args& ...furtherLocations) + : locations { location, furtherLocations... } + , m_what { + locations.size() == 1 + ? formatSingle(reason, locations[0]) + : formatMultiple(reason, locations) + } + { } /* Allows for the initialisation of an OpmInputError from another exception. @@ -83,27 +88,13 @@ public: } catch (const Opm::OpmInputError&) { throw; } catch (const std::exception& e) { - std::throw_with_nested(Opm::OpmInputError(location, e)); + std::throw_with_nested(Opm::OpmInputError(e, location)); } */ - OpmInputError(const KeywordLocation& loc, const std::exception& e) : - m_what { OpmInputError::formatException(loc, e) }, - locations { loc } - {} - - OpmInputError(const KeywordLocation& loc, const std::string& reason) - : m_what { OpmInputError::formatSingle(reason, loc) } - , locations { loc } - {} - - OpmInputError(const std::vector& locations, const std::string& reason) - : m_what { - locations.size() == 1 - ? OpmInputError::formatSingle(reason, locations[0]) - : OpmInputError::formatMultiple(reason, locations) - } - , locations { locations } - {} + OpmInputError(const std::exception& error, const KeywordLocation& location) + : locations { location } + , m_what { formatException(error, locations[0]) } + { } const char * what() const throw() { @@ -112,16 +103,16 @@ public: static std::string format(const std::string& msg_format, const KeywordLocation& loc); - static std::string formatException(const KeywordLocation& loc, const std::exception& e); + static std::string formatException(const std::exception& e, const KeywordLocation& loc); private: - std::string m_what; - // The location member is here for debugging; depending on the msg_fmt // passed in the constructor we might not have captured all the information // in the location argument passed to the constructor. std::vector locations; + std::string m_what; + static std::string formatSingle(const std::string& reason, const KeywordLocation&); static std::string formatMultiple(const std::string& reason, const std::vector&); }; diff --git a/src/opm/common/utility/OpmInputError.cpp b/src/opm/common/utility/OpmInputError.cpp index 221544fb5..bd75863c0 100644 --- a/src/opm/common/utility/OpmInputError.cpp +++ b/src/opm/common/utility/OpmInputError.cpp @@ -26,12 +26,26 @@ namespace Opm { -std::string OpmInputError::formatException(const KeywordLocation& loc, const std::exception& e) { - const std::string defaultMessage { R"(Problem parsing keyword {{keyword}} -In {{file}} line {{line}}. +namespace { + +template +std::string formatImpl(const std::string& msg_format, const KeywordLocation& loc, const Args& ...arguments) { + return fmt::format(msg_format, + arguments..., + fmt::arg("keyword", loc.keyword), + fmt::arg("file", loc.filename), + fmt::arg("line", loc.lineno) + ); +} + +} + +std::string OpmInputError::formatException(const std::exception& e, const KeywordLocation& loc) { + const std::string defaultMessage { R"(Problem parsing keyword {keyword} +In {file} line {line}. Internal error: {})" } ; - return format(fmt::format(defaultMessage, e.what()), loc); + return formatImpl(defaultMessage, loc, e.what()); } /* @@ -42,26 +56,23 @@ Internal error: {})" } ; the fmtlib dependendcy will be imposed on downstream modules. */ std::string OpmInputError::format(const std::string& msg_format, const KeywordLocation& loc) { - return fmt::format(msg_format, - fmt::arg("keyword", loc.keyword), - fmt::arg("file", loc.filename), - fmt::arg("line", loc.lineno) - ); + return formatImpl(msg_format, loc); +} + +std::string OpmInputError::formatSingle(const std::string& reason, const KeywordLocation& location) { + const std::string defaultMessage { R"(Problem parsing keyword {keyword} +In {file} line {line} +Parse error: {})" } ; + + return formatImpl(defaultMessage, location, reason); } namespace { - std::string locationStringLine(const KeywordLocation& loc) { - return OpmInputError::format("\n {keyword} in {file}, line {line}", loc); - } +std::string locationStringLine(const KeywordLocation& loc) { + return OpmInputError::format("\n {keyword} in {file}, line {line}", loc); } -std::string OpmInputError::formatSingle(const std::string& reason, const KeywordLocation& location) { - const std::string defaultMessage { R"(Problem parsing keyword {{keyword}} -In {{file}} line {{line}} -Parse error: {})" } ; - - return format(fmt::format(defaultMessage, reason), location); } std::string OpmInputError::formatMultiple(const std::string& reason, const std::vector& locations) { diff --git a/src/opm/parser/eclipse/EclipseState/Grid/FieldProps.cpp b/src/opm/parser/eclipse/EclipseState/Grid/FieldProps.cpp index d31f79df3..09287f230 100644 --- a/src/opm/parser/eclipse/EclipseState/Grid/FieldProps.cpp +++ b/src/opm/parser/eclipse/EclipseState/Grid/FieldProps.cpp @@ -861,7 +861,7 @@ void FieldProps::handle_operation(const DeckKeyword& keyword, Box box) { continue; } - throw OpmInputError(keyword.location(), "Operation keyword " + keyword.name() + " does not support the keyword " + target_kw); + throw OpmInputError("Operation keyword " + keyword.name() + " does not support the keyword " + target_kw, keyword.location()); } } diff --git a/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp b/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp index bffb35352..dd38c42bd 100644 --- a/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp +++ b/src/opm/parser/eclipse/EclipseState/Schedule/KeywordHandlers.cpp @@ -1767,7 +1767,7 @@ namespace { } catch (const OpmInputError&) { throw; } catch (const std::exception& e) { - OpmLog::error(OpmInputError::formatException(handlerContext.keyword.location(), e)); + OpmLog::error(OpmInputError::formatException(e, handlerContext.keyword.location())); throw; } diff --git a/src/opm/parser/eclipse/EclipseState/Tables/TableManager.cpp b/src/opm/parser/eclipse/EclipseState/Tables/TableManager.cpp index 9383be4e0..9c846fc6c 100644 --- a/src/opm/parser/eclipse/EclipseState/Tables/TableManager.cpp +++ b/src/opm/parser/eclipse/EclipseState/Tables/TableManager.cpp @@ -577,10 +577,7 @@ namespace Opm { hasRTEMPVD { deck.hasKeyword() } ; if (hasTEMPVD && hasRTEMPVD) { - throw OpmInputError({ - deck.getKeyword().location(), - deck.getKeyword().location(), - }, "The TEMPVD and RTEMPVD tables are mutually exclusive."); + throw OpmInputError("The TEMPVD and RTEMPVD tables are mutually exclusive.", deck.getKeyword().location(), deck.getKeyword().location()); } else if (hasTEMPVD) { initSimpleTableContainer(deck, "TEMPVD", "RTEMPVD", m_eqldims.getNumEquilRegions()); } else if (hasRTEMPVD) { @@ -636,7 +633,7 @@ namespace Opm { "The Parser does currently NOT support the alternating record schema used in PLYSHLOG" } ; - throw OpmInputError(tableKeyword.location(), reason); + throw OpmInputError(reason, tableKeyword.location()); } for (size_t tableIdx = 0; tableIdx < tableKeyword.size(); tableIdx += 2) { @@ -675,7 +672,7 @@ namespace Opm { " for keyword PLYMWINJ found" } ; - throw OpmInputError(keyword.location(), reason); + throw OpmInputError(reason, keyword.location()); } } } @@ -705,7 +702,7 @@ namespace Opm { " for keyword SKPRWAT found" } ; - throw OpmInputError(keyword.location(), reason); + throw OpmInputError(reason, keyword.location()); } } } @@ -735,7 +732,7 @@ namespace Opm { " for keyword SKPRPOLY found" } ; - throw OpmInputError(keyword.location(), reason); + throw OpmInputError(reason, keyword.location()); } } } @@ -807,7 +804,7 @@ namespace Opm { "Make sure that your ROCKTAB table only has 3 columns)" } ; - throw OpmInputError(keyword.location(), reason); + throw OpmInputError(reason, keyword.location()); } bool useStressOption = false; @@ -820,7 +817,7 @@ namespace Opm { if (useStressOption) { const std::string reason { "STRESS option is set in ROCKOPTS. Flow does not support stress option in rock compaction mulipliers" } ; - throw OpmInputError(rockoptsKeyword.location(), reason); + throw OpmInputError(reason, rockoptsKeyword.location()); } } diff --git a/src/opm/parser/eclipse/Parser/Parser.cpp b/src/opm/parser/eclipse/Parser/Parser.cpp index 233291b47..b5ca323a4 100644 --- a/src/opm/parser/eclipse/Parser/Parser.cpp +++ b/src/opm/parser/eclipse/Parser/Parser.cpp @@ -935,7 +935,7 @@ bool parseState( ParserState& parserState, const Parser& parser ) { exception. */ - OpmLog::error(OpmInputError::formatException(rawKeyword->location(), e)); + OpmLog::error(OpmInputError::formatException(e, rawKeyword->location())); throw; } diff --git a/tests/parser/ParseContextTests.cpp b/tests/parser/ParseContextTests.cpp index 2a0aef92b..54314eac4 100644 --- a/tests/parser/ParseContextTests.cpp +++ b/tests/parser/ParseContextTests.cpp @@ -848,9 +848,6 @@ BOOST_AUTO_TEST_CASE(OPM_ERROR) { OpmInputError error1("Error", location); OpmInputError error4("{keyword}:{line}:{keyword}", location); - BOOST_CHECK_EQUAL(error1.what(), "Error"); - BOOST_CHECK_EQUAL(error4.what(), "kw:100:kw"); - /* This test is meant to emulate the typical parsing process, the blocks here in the main test function represent main() in the simulator and the main diff --git a/tests/test_OpmInputError_format.cpp b/tests/test_OpmInputError_format.cpp index 87775a9d9..4eff08d70 100644 --- a/tests/test_OpmInputError_format.cpp +++ b/tests/test_OpmInputError_format.cpp @@ -54,7 +54,7 @@ BOOST_AUTO_TEST_CASE(exception) { In FILENAME.DAT line 42. Internal error: Runtime Error)" }; - const std::string formatted { Opm::OpmInputError::formatException(location, std::runtime_error("Runtime Error")) } ; + const std::string formatted { Opm::OpmInputError::formatException(std::runtime_error("Runtime Error"), location) } ; BOOST_CHECK_EQUAL(formatted, expected); } @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(exception_reason) { In FILENAME.DAT line 42. Internal error: Runtime Error)" }; - const std::string formatted { Opm::OpmInputError::formatException(location, std::runtime_error("Runtime Error")) } ; + const std::string formatted { Opm::OpmInputError::formatException(std::runtime_error("Runtime Error"), location) } ; BOOST_CHECK_EQUAL(formatted, expected); } @@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(exception_init) { In FILENAME.DAT line 42. Internal error: Runtime Error)" }; - const std::string formatted { Opm::OpmInputError(location, std::runtime_error("Runtime Error")).what() } ; + const std::string formatted { Opm::OpmInputError(std::runtime_error("Runtime Error"), location).what() } ; BOOST_CHECK_EQUAL(formatted, expected); } @@ -88,7 +88,7 @@ Internal error: Runtime Error)" }; try { throw std::runtime_error("Runtime Error"); } catch (const std::exception& e) { - std::throw_with_nested(Opm::OpmInputError(location, e)); + std::throw_with_nested(Opm::OpmInputError(e, location)); } } catch (const Opm::OpmInputError& opm_error) { BOOST_CHECK_EQUAL(opm_error.what(), expected); @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(exception_multi_1) { In FILENAME.DAT line 42 Parse error: Runtime Error)" } ; - const std::string formatted { Opm::OpmInputError({ location }, "Runtime Error").what() } ; + const std::string formatted { Opm::OpmInputError("Runtime Error", location).what() } ; BOOST_CHECK_EQUAL(formatted, expected); } @@ -113,7 +113,7 @@ BOOST_AUTO_TEST_CASE(exception_multi_2) { MZUNSUPP in FILENAME.DAT, line 45 Parse error: Runtime Error)" } ; - const std::string formatted { Opm::OpmInputError({ location, location2 }, "Runtime Error").what() } ; + const std::string formatted { Opm::OpmInputError("Runtime Error", location, location2).what() } ; BOOST_CHECK_EQUAL(formatted, expected); }