From: Teemu Murtola <teemu.murtola@gmail.com>
Date: Sat, 8 Nov 2014 10:39:28 +0000 (+0200)
Subject: Remove old selection parser error handling code
X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=ccc369c4e91bcec75a315dba7fcf818b1f3e761f;p=alexxy%2Fgromacs.git

Remove old selection parser error handling code

Now that the parser fully uses exceptions for error handling, the old
mechanism can go, simplifying the code in a few places.

Clean-up for #655.

Change-Id: I92c472a5298e89dce80e022d1e2101d822d90045
---

diff --git a/src/gromacs/options/options.cpp b/src/gromacs/options/options.cpp
index aec42fc98f..cd6732ec6b 100644
--- a/src/gromacs/options/options.cpp
+++ b/src/gromacs/options/options.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2014, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -48,7 +48,6 @@
 #include "gromacs/utility/arrayref.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/gmxassert.h"
-#include "gromacs/utility/messagestringcollector.h"
 #include "gromacs/utility/stringutil.h"
 
 #include "options-impl.h"
diff --git a/src/gromacs/selection/params.cpp b/src/gromacs/selection/params.cpp
index a246a6cb81..d4806d71eb 100644
--- a/src/gromacs/selection/params.cpp
+++ b/src/gromacs/selection/params.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -837,16 +837,15 @@ parse_values_std(const SelectionParserValueList &values,
             switch (value->type)
             {
                 case INT_VALUE:
+                {
+                    bool bTooManyValues;
                     if (value->u.i.i1 <= value->u.i.i2)
                     {
                         for (j = value->u.i.i1; j <= value->u.i.i2 && i < param->val.nr; ++j)
                         {
                             param->val.u.i[i++] = j;
                         }
-                        if (j != value->u.i.i2 + 1)
-                        {
-                            _gmx_selparser_error(scanner, "extra values skipped");
-                        }
+                        bTooManyValues = (j != value->u.i.i2 + 1);
                     }
                     else
                     {
@@ -854,13 +853,20 @@ parse_values_std(const SelectionParserValueList &values,
                         {
                             param->val.u.i[i++] = j;
                         }
-                        if (j != value->u.i.i2 - 1)
-                        {
-                            _gmx_selparser_error(scanner, "extra values skipped");
-                        }
+                        bTooManyValues = (j != value->u.i.i2 - 1);
+                    }
+                    if (bTooManyValues)
+                    {
+                        std::string text(_gmx_sel_lexer_get_text(scanner, value->location()));
+                        std::string message
+                            = formatString("Range ('%s') produces more values than is "
+                                           "accepted in this context",
+                                           text.c_str());
+                        GMX_THROW(InvalidInputError(message));
                     }
                     --i;
                     break;
+                }
                 case REAL_VALUE:
                     if (value->u.r.r1 != value->u.r.r2)
                     {
diff --git a/src/gromacs/selection/parsetree.cpp b/src/gromacs/selection/parsetree.cpp
index 239b2865b1..d3d9096f18 100644
--- a/src/gromacs/selection/parsetree.cpp
+++ b/src/gromacs/selection/parsetree.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -233,7 +233,6 @@
 #include "gromacs/utility/cstringutil.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/file.h"
-#include "gromacs/utility/messagestringcollector.h"
 #include "gromacs/utility/smalloc.h"
 #include "gromacs/utility/stringutil.h"
 
@@ -275,19 +274,6 @@ formatCurrentErrorContext(yyscan_t scanner)
 
 } // namespace
 
-void
-_gmx_selparser_error(yyscan_t scanner, const char *fmt, ...)
-{
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    // FIXME: Use an arbitrary length buffer.
-    char    buf[1024];
-    va_list ap;
-    va_start(ap, fmt);
-    vsnprintf(buf, 1024, fmt, ap);
-    va_end(ap);
-    errors->append(buf);
-}
-
 bool
 _gmx_selparser_handle_exception(yyscan_t scanner, std::exception *ex)
 {
@@ -659,9 +645,6 @@ _gmx_sel_init_comparison(const gmx::SelectionTreeElementPointer &left,
                          const gmx::SelectionTreeElementPointer &right,
                          const char *cmpop, yyscan_t scanner)
 {
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    gmx::MessageStringContext    context(errors, formatCurrentErrorContext(scanner));
-
     SelectionTreeElementPointer  sel(
             new SelectionTreeElement(
                     SEL_EXPRESSION, _gmx_sel_lexer_get_current_location(scanner)));
@@ -710,9 +693,6 @@ init_keyword_internal(gmx_ana_selmethod_t *method,
 {
     gmx_ana_selcollection_t     *sc = _gmx_sel_lexer_selcollection(scanner);
 
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    gmx::MessageStringContext    context(errors, formatCurrentErrorContext(scanner));
-
     if (method->nparams > 0)
     {
         // TODO: Would assert be better?
@@ -818,9 +798,7 @@ _gmx_sel_init_keyword_of(gmx_ana_selmethod_t                    *method,
                          const gmx::SelectionTreeElementPointer &group,
                          const char *rpost, yyscan_t scanner)
 {
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    gmx::MessageStringContext    context(errors, formatCurrentErrorContext(scanner));
-
+    // TODO Provide an error if rpost is provided.
     GMX_UNUSED_VALUE(rpost);
     return _gmx_sel_init_keyword_evaluator(method, group, scanner);
 }
@@ -847,9 +825,6 @@ _gmx_sel_init_method(gmx_ana_selmethod_t                      *method,
 {
     gmx_ana_selcollection_t     *sc = _gmx_sel_lexer_selcollection(scanner);
 
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    gmx::MessageStringContext    context(errors, formatCurrentErrorContext(scanner));
-
     _gmx_sel_finish_method(scanner);
     /* The "same" keyword needs some custom massaging of the parameters. */
     _gmx_selelem_custom_init_same(&method, params, scanner);
@@ -881,9 +856,6 @@ _gmx_sel_init_modifier(gmx_ana_selmethod_t                      *method,
                        const gmx::SelectionTreeElementPointer   &sel,
                        yyscan_t                                  scanner)
 {
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    gmx::MessageStringContext    context(errors, formatCurrentErrorContext(scanner));
-
     _gmx_sel_finish_method(scanner);
     SelectionTreeElementPointer modifier(
             new SelectionTreeElement(
@@ -926,9 +898,6 @@ SelectionTreeElementPointer
 _gmx_sel_init_position(const gmx::SelectionTreeElementPointer &expr,
                        const char *type, yyscan_t scanner)
 {
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    gmx::MessageStringContext    context(errors, formatCurrentErrorContext(scanner));
-
     SelectionTreeElementPointer  root(
             new SelectionTreeElement(
                     SEL_EXPRESSION, _gmx_sel_lexer_get_current_location(scanner)));
diff --git a/src/gromacs/selection/parsetree.h b/src/gromacs/selection/parsetree.h
index 901795e49b..3e0d5f49e4 100644
--- a/src/gromacs/selection/parsetree.h
+++ b/src/gromacs/selection/parsetree.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -409,9 +409,6 @@ class SelectionParserParameter
 
 } // namespace gmx
 
-/** Error reporting function for the selection parser. */
-void
-_gmx_selparser_error(void *scanner, const char *fmt, ...);
 /*! \brief
  * Handles exceptions caught within the Bison code.
  *
diff --git a/src/gromacs/selection/scanner.h b/src/gromacs/selection/scanner.h
index b49a445365..ad7fcf56ce 100644
--- a/src/gromacs/selection/scanner.h
+++ b/src/gromacs/selection/scanner.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -51,11 +51,6 @@
 
 #include "parser.h"
 
-namespace gmx
-{
-class MessageStringCollector;
-}
-
 struct gmx_ana_indexgrps_t;
 struct gmx_ana_selcollection_t;
 
@@ -72,10 +67,6 @@ _gmx_sel_init_lexer(yyscan_t *scannerp, struct gmx_ana_selcollection_t *sc,
 /** Frees memory allocated for the selection scanner. */
 void
 _gmx_sel_free_lexer(yyscan_t scanner);
-/** Sets the error reporter object for the selection scanner. */
-void
-_gmx_sel_set_lexer_error_reporter(yyscan_t                     scanner,
-                                  gmx::MessageStringCollector *errors);
 /** Stores an exception that is caught during parsing. */
 void
 _gmx_sel_lexer_set_exception(yyscan_t                    scanner,
@@ -92,9 +83,6 @@ _gmx_sel_is_lexer_interactive(yyscan_t scanner);
 /** Returns the selection collection for the scanner. */
 struct gmx_ana_selcollection_t *
 _gmx_sel_lexer_selcollection(yyscan_t scanner);
-/** Returns the error reporter for the scanner. */
-gmx::MessageStringCollector *
-_gmx_sel_lexer_error_reporter(yyscan_t scanner);
 /** Returns true if the external index groups for the scanner are set. */
 bool
 _gmx_sel_lexer_has_groups_set(yyscan_t scanner);
diff --git a/src/gromacs/selection/scanner_internal.cpp b/src/gromacs/selection/scanner_internal.cpp
index 714a22ad1f..fc20e46890 100644
--- a/src/gromacs/selection/scanner_internal.cpp
+++ b/src/gromacs/selection/scanner_internal.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -63,7 +63,6 @@
 #include "gromacs/utility/cstringutil.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/gmxassert.h"
-#include "gromacs/utility/messagestringcollector.h"
 #include "gromacs/utility/smalloc.h"
 #include "gromacs/utility/stringutil.h"
 
@@ -398,7 +397,6 @@ _gmx_sel_init_lexer(yyscan_t *scannerp, struct gmx_ana_selcollection_t *sc,
     gmx_sel_lexer_t *state = new gmx_sel_lexer_t;
 
     state->sc        = sc;
-    state->errors    = NULL;
     state->bGroups   = bGroups;
     state->grps      = grps;
     state->nexpsel   = (maxnr > 0 ? static_cast<int>(sc->sel.size()) + maxnr : -1);
@@ -443,14 +441,6 @@ _gmx_sel_free_lexer(yyscan_t scanner)
     _gmx_sel_yylex_destroy(scanner);
 }
 
-void
-_gmx_sel_set_lexer_error_reporter(yyscan_t                     scanner,
-                                  gmx::MessageStringCollector *errors)
-{
-    gmx_sel_lexer_t *state = _gmx_sel_yyget_extra(scanner);
-    state->errors = errors;
-}
-
 void
 _gmx_sel_lexer_set_exception(yyscan_t                    scanner,
                              const boost::exception_ptr &ex)
@@ -485,14 +475,6 @@ _gmx_sel_lexer_selcollection(yyscan_t scanner)
     return state->sc;
 }
 
-gmx::MessageStringCollector *
-_gmx_sel_lexer_error_reporter(yyscan_t scanner)
-{
-    gmx_sel_lexer_t *state = _gmx_sel_yyget_extra(scanner);
-    GMX_RELEASE_ASSERT(state->errors != NULL, "Error reporter not set");
-    return state->errors;
-}
-
 bool
 _gmx_sel_lexer_has_groups_set(yyscan_t scanner)
 {
diff --git a/src/gromacs/selection/scanner_internal.h b/src/gromacs/selection/scanner_internal.h
index 9a407cc9a7..5bd8eb0e43 100644
--- a/src/gromacs/selection/scanner_internal.h
+++ b/src/gromacs/selection/scanner_internal.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -47,7 +47,6 @@
 
 namespace gmx
 {
-class MessageStringCollector;
 class SelectionParserSymbol;
 }
 
@@ -73,8 +72,6 @@ typedef struct gmx_sel_lexer_t
 {
     //! Selection collection to put parsed selections in.
     struct gmx_ana_selcollection_t  *sc;
-    //! Error reporter object.
-    gmx::MessageStringCollector     *errors;
     //! Stores an exception that occurred during parsing.
     boost::exception_ptr             exception;
     //! Whether external index groups have been set.
diff --git a/src/gromacs/selection/selectioncollection.cpp b/src/gromacs/selection/selectioncollection.cpp
index e465ed69a0..e18078814b 100644
--- a/src/gromacs/selection/selectioncollection.cpp
+++ b/src/gromacs/selection/selectioncollection.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -63,7 +63,6 @@
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/file.h"
 #include "gromacs/utility/gmxassert.h"
-#include "gromacs/utility/messagestringcollector.h"
 #include "gromacs/utility/smalloc.h"
 #include "gromacs/utility/stringutil.h"
 
@@ -338,11 +337,7 @@ SelectionList runParser(yyscan_t scanner, bool bStdIn, int maxnr,
     gmx_ana_selcollection_t *sc   = _gmx_sel_lexer_selcollection(scanner);
     gmx_ana_indexgrps_t     *grps = _gmx_sel_lexer_indexgrps(scanner);
 
-    MessageStringCollector   errors;
-    _gmx_sel_set_lexer_error_reporter(scanner, &errors);
-
-    size_t oldCount = sc->sel.size();
-    bool   bOk      = false;
+    size_t                   oldCount = sc->sel.size();
     {
         boost::shared_ptr<_gmx_sel_yypstate> parserState(
                 _gmx_sel_yypstate_new(), &_gmx_sel_yypstate_delete);
@@ -382,40 +377,33 @@ SelectionList runParser(yyscan_t scanner, bool bStdIn, int maxnr,
                     // error/warning if some input was ignored.
                     goto early_termination;
                 }
-                if (!errors.isEmpty() && bInteractive)
-                {
-                    fprintf(stderr, "%s", errors.toString().c_str());
-                    errors.clear();
-                }
             }
             {
                 YYLTYPE location;
                 status = _gmx_sel_yypush_parse(parserState.get(), 0, NULL,
                                                &location, scanner);
             }
+            // TODO: Remove added selections from the collection if parsing failed?
             _gmx_sel_lexer_rethrow_exception_if_occurred(scanner);
 early_termination:
-            bOk = (status == 0);
+            GMX_RELEASE_ASSERT(status == 0,
+                               "Parser errors should have resulted in an exception");
         }
         else
         {
             int status = runParserLoop(scanner, parserState.get(), false);
-            bOk = (status == 0);
+            GMX_RELEASE_ASSERT(status == 0,
+                               "Parser errors should have resulted in an exception");
         }
     }
     scannerGuard.reset();
     int nr = sc->sel.size() - oldCount;
     if (maxnr > 0 && nr != maxnr)
     {
-        bOk = false;
-        errors.append("Too few selections provided");
-    }
-
-    // TODO: Remove added selections from the collection if parsing failed?
-    if (!bOk || !errors.isEmpty())
-    {
-        GMX_ASSERT(!bOk && !errors.isEmpty(), "Inconsistent error reporting");
-        GMX_THROW(InvalidInputError(errors.toString()));
+        std::string message
+            = formatString("Too few selections provided; got %d, expected %d",
+                           nr, maxnr);
+        GMX_THROW(InvalidInputError(message));
     }
 
     SelectionList                     result;
diff --git a/src/gromacs/selection/selectionoption.cpp b/src/gromacs/selection/selectionoption.cpp
index f278723cc1..019f8965b5 100644
--- a/src/gromacs/selection/selectionoption.cpp
+++ b/src/gromacs/selection/selectionoption.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -151,6 +151,7 @@ void SelectionOptionStorage::processAll()
 void SelectionOptionStorage::setAllowedValueCount(int count)
 {
     // TODO: It should be possible to have strong exception safety here.
+    // TODO: Use ExceptionInitializer here.
     MessageStringCollector errors;
     errors.startContext("In option '" + name() + "'");
     if (count >= 0)
diff --git a/src/gromacs/selection/sm_keywords.cpp b/src/gromacs/selection/sm_keywords.cpp
index 698aa2bbc3..ef063d7463 100644
--- a/src/gromacs/selection/sm_keywords.cpp
+++ b/src/gromacs/selection/sm_keywords.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2009,2010,2011,2012,2013,2014, by the GROMACS development team, led by
+ * Copyright (c) 2009,2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -55,7 +55,6 @@
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/gmxassert.h"
 #include "gromacs/utility/gmxregex.h"
-#include "gromacs/utility/messagestringcollector.h"
 #include "gromacs/utility/smalloc.h"
 #include "gromacs/utility/stringutil.h"
 
@@ -861,11 +860,6 @@ _gmx_sel_init_keyword_evaluator(gmx_ana_selmethod_t                    *method,
                                 const gmx::SelectionTreeElementPointer &child,
                                 void                                   *scanner)
 {
-    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
-    char  buf[1024];
-    sprintf(buf, "In evaluation of '%s'", method->name);
-    gmx::MessageStringContext            context(errors, buf);
-
     gmx::SelectionParserParameterList    params;
     params.push_back(
             gmx::SelectionParserParameter::createFromExpression(NULL, child));