[lmi-commits] [lmi] master 97d2acd: Throw almost always on input-sequence parser error

[Greg Chicares]

branch: master
commit 97d2acdd0159343881b4ab22ed2c0185a1f4194f
Author: Gregory W. Chicares <address@hidden>
Commit: Gregory W. Chicares <address@hidden>

    Throw almost always on input-sequence parser error
    
    The parser emits diagnostics only when it detects a problem. Formerly,
    diagnostics were often ignored. E.g., if constructing an InputSequence
    requires calling the parser, the ctor simply saves the diagnostics and
    calls realize_vector()--which returns after preliminary initialization
    (without doing its real job) if there were any diagnostics.
    
    Downstream clients use any of the *representation() functions to access
    the supposedly parsed sequence. In theory, they should first ensure that
    the diagnostics are empty, but such has not been the universal practice.
    Thus, invalid sequences were treated as valid, in 'input_test.cpp' for
    example, where must_overwrite_specamt_with_obsolete_history() was called
    to compare two sequences and deemed them equal only because one was
    invalid (ignoring diagnostics made that sequence look like all zeros).
    
    Ultimately, diagnostics must always raise exceptions, because the parser
    doesn't issue mere benign warnings--it complains only of frank errors.
    This commit is a preliminary step toward that ultimate goal: it prevents
    clients from using an invalid InputSequence if they access it through
    any *representation() function but interval_representation(). That last
    case will be handled soon, but it calls for careful manual testing
    because it's the representation used by the sequence-editor GUI.
    
    Once all *representation() functions throw on nonempty diagnostics, the
    throw-points can all be moved up into the constructor, making parser
    success (empty diagnostics) a post-construction invariant.
---
 input_realization.cpp |   16 ++++++++++++++++
 input_sequence.cpp    |   15 +++++++++++++++
 input_test.cpp        |   12 +++++++-----
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/input_realization.cpp b/input_realization.cpp
index 0524b22..6335b31 100644
--- a/input_realization.cpp
+++ b/input_realization.cpp
@@ -36,6 +36,7 @@
 #include "value_cast.hpp"
 
 #include <algorithm>
+#include <exception>
 #include <functional>                   // std::bind()
 #include <sstream>
 #include <utility>                      // std::pair
@@ -929,6 +930,21 @@ int Input::must_overwrite_specamt_with_obsolete_history
         realize_sequence_string(*this, u, numeric_sequence(specamt));
         realize_sequence_string(*this, v, numeric_sequence(history));
         }
+    catch(std::exception const& e)
+        {
+        if(!hide_errors)
+            {
+            warning()
+                << "Possible conflict between specified amount and history."
+                << " Merge them manually into the specified-amount field."
+                << "\nSpecified amount: " << specamt
+                << "\nHistory: " << history
+                << "\nError: " << e.what()
+                << LMI_FLUSH
+                ;
+            }
+        return 2;
+        }
     catch(...)
         {
         report_exception();
diff --git a/input_sequence.cpp b/input_sequence.cpp
index 0e08fe5..ae73a54 100644
--- a/input_sequence.cpp
+++ b/input_sequence.cpp
@@ -918,11 +918,21 @@ InputSequence::~InputSequence() = default;
 
 std::vector<double> const& InputSequence::linear_number_representation() const
 {
+    if(!formatted_diagnostics().empty())
+        {
+        fatal_error() << formatted_diagnostics() << LMI_FLUSH;
+        }
+
     return number_result_;
 }
 
 std::vector<std::string> const& InputSequence::linear_keyword_representation() 
const
 {
+    if(!formatted_diagnostics().empty())
+        {
+        fatal_error() << formatted_diagnostics() << LMI_FLUSH;
+        }
+
     return keyword_result_;
 }
 
@@ -945,6 +955,11 @@ std::vector<std::string> const& 
InputSequence::linear_keyword_representation() c
 
 std::string InputSequence::mathematical_representation() const
 {
+    if(!formatted_diagnostics().empty())
+        {
+        fatal_error() << formatted_diagnostics() << LMI_FLUSH;
+        }
+
     std::ostringstream oss;
     for(auto const& interval_i : intervals_)
         {
diff --git a/input_test.cpp b/input_test.cpp
index 80beff2..b7f4b95 100644
--- a/input_test.cpp
+++ b/input_test.cpp
@@ -388,11 +388,13 @@ void input_test::test_obsolete_history()
     // This would have been forbidden: history was numeric only.
 //  BOOST_TEST(2 == z.must_overwrite_specamt_with_obsolete_history("X"    , 
"1"           , true));
     BOOST_TEST(2 == z.must_overwrite_specamt_with_obsolete_history("1"    , 
"X"           , true));
-    // This case differs little from the preceding one, but has a different
-    // outcome. Reason: non-numeric substrings are interepreted as zero.
-    // This oddity doesn't matter, because specified amount by its nature
-    // must be positive, and zero is returned so that nothing is changed.
-    BOOST_TEST(0 == z.must_overwrite_specamt_with_obsolete_history("0"    , 
"X"           , true));
+    // This case differs little from the preceding one, but formerly it
+    // had a different outcome. Reason: non-numeric substrings were
+    // interepreted as zero, simply because realize_vector() exited
+    // early if the input-sequence parser emitted any diagnostics. Now,
+    // however, any parser diagnostic raises an exception, which causes
+    // must_overwrite_specamt_with_obsolete_history() to return 2.
+    BOOST_TEST(2 == z.must_overwrite_specamt_with_obsolete_history("0"    , 
"X"           , true));
 }
 
 void input_test::assay_speed()