Merge release-5-0 into master
authorTeemu Murtola <teemu.murtola@gmail.com>
Fri, 22 May 2015 10:45:31 +0000 (13:45 +0300)
committerTeemu Murtola <teemu.murtola@gmail.com>
Fri, 22 May 2015 10:46:40 +0000 (13:46 +0300)
Conflicts:
    src/gromacs/selection/tests/selectioncollection.cpp

Took both tests added in the different branches at the same location.

Change-Id: I305dd8e37373c3f913b3e86d0ddb286926c3d0db

1  2 
src/gromacs/selection/params.cpp
src/gromacs/selection/sm_same.cpp
src/gromacs/selection/tests/selectioncollection.cpp

index d4806d71eb57b5522550e2dab58a6388c17b5f53,fb462861d85d371d9654a87b30e535945570d12d..bf387200f1dab898b3bd8896aacf1e60059d240a
   * \author Teemu Murtola <teemu.murtola@gmail.com>
   * \ingroup module_selection
   */
 +#include "gmxpre.h"
 +
  #include <algorithm>
  #include <string>
  
 -#include "gromacs/legacyheaders/vec.h"
 -
 +#include "gromacs/math/vec.h"
  #include "gromacs/selection/position.h"
 -#include "gromacs/selection/selmethod.h"
 -#include "gromacs/selection/selparam.h"
 +#include "gromacs/utility/cstringutil.h"
  #include "gromacs/utility/exceptions.h"
  #include "gromacs/utility/gmxassert.h"
 -#include "gromacs/utility/messagestringcollector.h"
 +#include "gromacs/utility/scoped_cptr.h"
  #include "gromacs/utility/smalloc.h"
  #include "gromacs/utility/stringutil.h"
  
  #include "parsetree.h"
 -#include "position.h"
  #include "scanner.h"
  #include "selelem.h"
 +#include "selmethod.h"
 +#include "selparam.h"
  
 -using gmx::SelectionParserValue;
 -using gmx::SelectionParserValueList;
 -using gmx::SelectionParserParameter;
 -using gmx::SelectionParserParameterList;
 -using gmx::SelectionTreeElement;
 -using gmx::SelectionTreeElementPointer;
 +using namespace gmx;
  
  /*!
   * \param[in] name   Name of the parameter to search.
@@@ -112,16 -116,15 +112,16 @@@ gmx_ana_selparam_find(const char *name
   *
   * \param[in,out] value    Value to convert.
   * \param[in]     type     Type to convert to.
 + * \param[in]     errors   Errors will be reported into this as nested exceptions.
   * \param[in]     scanner  Scanner data structure.
 - * \returns       0 on success, a non-zero value on error.
   */
 -static int
 -convert_value(SelectionParserValue *value, e_selvalue_t type, void *scanner)
 +static void
 +convert_value(SelectionParserValue *value, e_selvalue_t type,
 +              ExceptionInitializer *errors, void *scanner)
  {
      if (value->type == type || type == NO_VALUE)
      {
 -        return 0;
 +        return;
      }
      if (value->hasExpressionValue())
      {
           * reference positions. */
          if (value->type == GROUP_VALUE && type == POS_VALUE)
          {
 -            SelectionTreeElementPointer expr =
 -                _gmx_sel_init_position(value->expr, NULL, scanner);
 -            // FIXME: Use exceptions
 -            if (!expr)
 +            try
 +            {
 +                SelectionTreeElementPointer expr =
 +                    _gmx_sel_init_position(value->expr, NULL, scanner);
 +                *value = SelectionParserValue::createExpr(expr);
 +            }
 +            catch (UserInputError &ex)
              {
 -                return -1;
 +                std::string text(_gmx_sel_lexer_get_text(scanner, value->location()));
 +                std::string context(formatString("In '%s'", text.c_str()));
 +                ex.prependContext(context);
 +                errors->addCurrentExceptionAsNested();
              }
 -            *value = SelectionParserValue::createExpr(expr);
 -            return 0;
 +            return;
          }
 -        return -1;
      }
      else
      {
          if (value->type == INT_VALUE && type == REAL_VALUE)
          {
              *value = SelectionParserValue::createRealRange(value->u.i.i1,
 -                                                           value->u.i.i2);
 -            return 0;
 +                                                           value->u.i.i2,
 +                                                           value->location());
 +            return;
          }
          /* Reals that are integer-valued can also be converted */
          if (value->type == REAL_VALUE && type == INT_VALUE)
              if (gmx_within_tol(value->u.r.r1, i1, GMX_REAL_EPS)
                  && gmx_within_tol(value->u.r.r2, i2, GMX_REAL_EPS))
              {
 -                *value = SelectionParserValue::createIntegerRange(i1, i2);
 -                return 0;
 +                *value = SelectionParserValue::createIntegerRange(i1, i2, value->location());
 +                return;
              }
          }
      }
 -    return -1;
 +    std::string text(_gmx_sel_lexer_get_text(scanner, value->location()));
 +    std::string message(
 +            formatString("Expression '%s' evaluates to a type is not valid in this context",
 +                         text.c_str()));
 +    InvalidInputError ex(message);
 +    errors->addNested(ex);
  }
  
  /*! \brief
   * \param[in,out] values   Values to convert.
   * \param[in]     type     Type to convert to.
   * \param[in]     scanner  Scanner data structure.
 - * \returns       0 on success, a non-zero value on error.
   */
 -static int
 +static void
  convert_values(SelectionParserValueList *values, e_selvalue_t type, void *scanner)
  {
 -    int rc = 0;
 +    ExceptionInitializer               errors("");
      SelectionParserValueList::iterator value;
      for (value = values->begin(); value != values->end(); ++value)
      {
 -        int rc1 = convert_value(&*value, type, scanner);
 -        if (rc1 != 0 && rc == 0)
 -        {
 -            rc = rc1;
 -        }
 +        convert_value(&*value, type, &errors, scanner);
 +    }
 +    if (errors.hasNestedExceptions())
 +    {
 +        GMX_THROW(InvalidInputError(errors));
      }
 -    /* FIXME: More informative error messages */
 -    return rc;
  }
  
  /*! \brief
@@@ -298,44 -294,40 +298,44 @@@ cmp_real_range(const void *a, const voi
   * \param[in] values List of values.
   * \param     param  Parameter to parse.
   * \param[in] scanner Scanner data structure.
 - * \returns   true if the values were parsed successfully, false otherwise.
   */
 -static bool
 +static void
  parse_values_range(const SelectionParserValueList &values,
                     gmx_ana_selparam_t *param, void *scanner)
  {
 -    int                *idata;
 -    real               *rdata;
      int                 i, j, n;
  
      param->flags &= ~SPAR_DYNAMIC;
      GMX_RELEASE_ASSERT(param->val.type == INT_VALUE || param->val.type == REAL_VALUE,
                         "Invalid range parameter type");
 -    idata = NULL;
 -    rdata = NULL;
 +    int                *idata = NULL;
 +    real               *rdata = NULL;
 +    scoped_guard_sfree  dataGuard;
      if (param->val.type == INT_VALUE)
      {
          snew(idata, values.size()*2);
 +        dataGuard.reset(idata);
      }
      else
      {
          snew(rdata, values.size()*2);
 +        dataGuard.reset(rdata);
      }
      i = 0;
      SelectionParserValueList::const_iterator value;
      for (value = values.begin(); value != values.end(); ++value)
      {
 +        GMX_RELEASE_ASSERT(value->type == param->val.type,
 +                           "Invalid range value type (should have been caught earlier)");
          if (value->hasExpressionValue())
          {
 -            _gmx_selparser_error(scanner, "expressions not supported within range parameters");
 -            return false;
 +            std::string       text(_gmx_sel_lexer_get_text(scanner, value->location()));
 +            std::string       message("Only simple values or 'A to B' ranges are "
 +                                      "supported in this context");
 +            InvalidInputError ex(message);
 +            ex.prependContext(formatString("Invalid expression '%s'", text.c_str()));
 +            GMX_THROW(ex);
          }
 -        GMX_RELEASE_ASSERT(value->type == param->val.type,
 -                           "Invalid range value type (should have been caught earlier)");
          if (param->val.type == INT_VALUE)
          {
              int i1 = std::min(value->u.i.i1, value->u.i.i2);
          qsort(rdata, n, 2*sizeof(real), &cmp_real_range);
          for (i = j = 2; i < 2*n; i += 2)
          {
-             if (rdata[j-1]+1 >= rdata[i])
+             if (rdata[j-1] >= rdata[i])
              {
                  if (rdata[i+1] > rdata[j-1])
                  {
      /* Store the values */
      if (param->flags & SPAR_VARNUM)
      {
 +        dataGuard.release();
          param->val.nr  = n;
          if (param->val.type == INT_VALUE)
          {
      {
          if (n != param->val.nr)
          {
 -            _gmx_selparser_error(scanner, "the value should consist of exactly one range");
 -            sfree(idata);
 -            sfree(rdata);
 -            return false;
 +            GMX_ASSERT(n == 1,
 +                       "Range parameters with a fixed count > 1 do not make sense");
 +            GMX_THROW(InvalidInputError("Only one value or 'A to B' range is "
 +                                        "supported in this context"));
          }
          if (param->val.type == INT_VALUE)
          {
              memcpy(param->val.u.i, idata, 2*n*sizeof(int));
 -            sfree(idata);
          }
          else
          {
              memcpy(param->val.u.r, rdata, 2*n*sizeof(real));
 -            sfree(rdata);
          }
      }
      if (param->nvalptr)
          *param->nvalptr = param->val.nr;
      }
      param->nvalptr = NULL;
 -
 -    return true;
  }
  
  /*! \brief
   * \param     param  Parameter to parse.
   * \param     root   Selection element to which child expressions are added.
   * \param[in] scanner Scanner data structure.
 - * \returns   true if the values were parsed successfully, false otherwise.
   *
   * For integer ranges, the sequence of numbers from the first to second value
   * is stored, each as a separate value.
   */
 -static bool
 +static void
  parse_values_varnum(const SelectionParserValueList    &values,
                      gmx_ana_selparam_t                *param,
                      const SelectionTreeElementPointer &root,
  
      param->flags &= ~SPAR_DYNAMIC;
      /* Compute number of values, considering also integer ranges. */
 -    size_t valueCount = values.size();
 +    int valueCount = static_cast<int>(values.size());
      if (param->val.type == INT_VALUE)
      {
          SelectionParserValueList::const_iterator value;
      if (param->val.type != INT_VALUE && param->val.type != REAL_VALUE
          && param->val.type != STR_VALUE && param->val.type != POS_VALUE)
      {
 -        GMX_THROW(gmx::InternalError("Variable-count value type not implemented"));
 +        GMX_THROW(InternalError("Variable-count value type not implemented"));
      }
  
      /* Reserve appropriate amount of memory */
      {
          _gmx_selvalue_reserve(&param->val, valueCount);
      }
 +    /* Create a dummy child element to store the string values.
 +     * This element is responsible for freeing the values, but carries no
 +     * other function. */
 +    if (param->val.type == STR_VALUE)
 +    {
 +        SelectionTreeElementPointer child(
 +                new SelectionTreeElement(SEL_CONST, SelectionLocation::createEmpty()));
 +        _gmx_selelem_set_vtype(child, STR_VALUE);
 +        child->setName(param->name);
 +        child->flags &= ~SEL_ALLOCVAL;
 +        child->flags |= SEL_FLAGSSET | SEL_VARNUMVAL | SEL_ALLOCDATA;
 +        child->v.nr   = valueCount;
 +        _gmx_selvalue_setstore(&child->v, param->val.u.s);
 +        /* Because the child is not group-valued, the u union is not used
 +         * for anything, so we can abuse it by storing the parameter value
 +         * as place_child() expects, but this is really ugly... */
 +        child->u.param = param;
 +        place_child(root, child, param);
 +    }
 +    param->val.nr = valueCount;
  
      i     = 0;
      SelectionParserValueList::const_iterator value;
      for (value = values.begin(); value != values.end(); ++value)
      {
 +        GMX_RELEASE_ASSERT(value->type == param->val.type,
 +                           "Invalid value type (should have been caught earlier)");
          if (value->hasExpressionValue())
          {
 -            _gmx_selparser_error(scanner, "expressions not supported within value lists");
 -            return false;
 +            std::string       text(_gmx_sel_lexer_get_text(scanner, value->location()));
 +            std::string       message("Selection expressions are not supported in this "
 +                                      "context when multiple values are provided");
 +            InvalidInputError ex(message);
 +            ex.prependContext(formatString("Invalid expression '%s'", text.c_str()));
 +            GMX_THROW(ex);
          }
 -        GMX_RELEASE_ASSERT(value->type == param->val.type,
 -                           "Invalid value type (should have been caught earlier)");
          switch (param->val.type)
          {
              case INT_VALUE:
              case REAL_VALUE:
                  if (value->u.r.r1 != value->u.r.r2)
                  {
 -                    _gmx_selparser_error(scanner, "real ranges not supported");
 -                    return false;
 +                    std::string text(_gmx_sel_lexer_get_text(scanner, value->location()));
 +                    std::string message
 +                        = formatString("Real range ('%s') is not supported in this context",
 +                                       text.c_str());
 +                    InvalidInputError ex(message);
 +                    GMX_THROW(ex);
                  }
                  param->val.u.r[i++] = value->u.r.r1;
                  break;
              case STR_VALUE:
 -                param->val.u.s[i++] = strdup(value->stringValue().c_str());
 +                param->val.u.s[i++] = gmx_strdup(value->stringValue().c_str());
                  break;
              case POS_VALUE:  copy_rvec(value->u.x, param->val.u.p->x[i++]); break;
              default: /* Should not be reached */
 -                GMX_THROW(gmx::InternalError("Variable-count value type not implemented"));
 +                GMX_RELEASE_ASSERT(false, "Variable-count value type not implemented");
          }
      }
 -    param->val.nr = i;
 +    GMX_RELEASE_ASSERT(i == valueCount,
 +                       "Inconsistent value count wrt. the actual value population");
      if (param->nvalptr)
      {
          *param->nvalptr = param->val.nr;
      }
      param->nvalptr = NULL;
 -    /* Create a dummy child element to store the string values.
 -     * This element is responsible for freeing the values, but carries no
 -     * other function. */
 -    if (param->val.type == STR_VALUE)
 -    {
 -        SelectionTreeElementPointer child(new SelectionTreeElement(SEL_CONST));
 -        _gmx_selelem_set_vtype(child, STR_VALUE);
 -        child->setName(param->name);
 -        child->flags &= ~SEL_ALLOCVAL;
 -        child->flags |= SEL_FLAGSSET | SEL_VARNUMVAL | SEL_ALLOCDATA;
 -        child->v.nr   = param->val.nr;
 -        _gmx_selvalue_setstore(&child->v, param->val.u.s);
 -        /* Because the child is not group-valued, the u union is not used
 -         * for anything, so we can abuse it by storing the parameter value
 -         * as place_child() expects, but this is really ugly... */
 -        child->u.param = param;
 -        place_child(root, child, param);
 -    }
 -
 -    return true;
  }
  
  /*! \brief
@@@ -616,8 -603,7 +616,8 @@@ add_child(const SelectionTreeElementPoi
      }
      else
      {
 -        child.reset(new SelectionTreeElement(SEL_SUBEXPRREF));
 +        // TODO: Initialize such that it includes the parameter.
 +        child.reset(new SelectionTreeElement(SEL_SUBEXPRREF, expr->location()));
          _gmx_selelem_set_vtype(child, expr->v.type);
          child->child  = expr;
      }
      child->u.param = param;
      if (child->v.type != param->val.type)
      {
 -        _gmx_selparser_error(scanner, "invalid expression value");
 -        // FIXME: Use exceptions.
 -        return SelectionTreeElementPointer();
 +        // TODO: It would be nice to say what is the expected type.
 +        std::string text(_gmx_sel_lexer_get_text(scanner, expr->location()));
 +        std::string message
 +            = formatString("Expression '%s' is not valid in this context "
 +                           "(produces the wrong type of values)",
 +                           text.c_str());
 +        GMX_THROW(InvalidInputError(message));
      }
      _gmx_selelem_update_flags(child);
      if ((child->flags & SEL_DYNAMIC) && !(param->flags & SPAR_DYNAMIC))
      {
 -        _gmx_selparser_error(scanner, "dynamic values not supported");
 -        // FIXME: Use exceptions.
 -        return SelectionTreeElementPointer();
 +        std::string text(_gmx_sel_lexer_get_text(scanner, expr->location()));
 +        std::string message
 +            = formatString("Expression '%s' is dynamic, which is not "
 +                           "valid in this context",
 +                           text.c_str());
 +        GMX_THROW(InvalidInputError(message));
      }
      if (!(child->flags & SEL_DYNAMIC))
      {
   * \param     param  Parameter to parse.
   * \param     root   Selection element to which child expressions are added.
   * \param[in] scanner Scanner data structure.
 - * \returns   true if the values were parsed successfully, false otherwise.
   */
 -static bool
 +static void
  parse_values_varnum_expr(const SelectionParserValueList    &values,
                           gmx_ana_selparam_t                *param,
                           const SelectionTreeElementPointer &root,
  
      SelectionTreeElementPointer child
          = add_child(root, param, values.front().expr, scanner);
 -    if (!child)
 -    {
 -        return false;
 -    }
  
      /* Process single-valued expressions */
      /* TODO: We should also handle SEL_SINGLEVAL expressions here */
              *param->nvalptr = param->val.nr;
          }
          param->nvalptr = NULL;
 -        return true;
 +        return;
      }
  
      if (!(child->flags & SEL_VARNUMVAL))
      {
 -        _gmx_selparser_error(scanner, "invalid expression value");
 -        return false;
 +        std::string text(_gmx_sel_lexer_get_text(scanner, values.front().location()));
 +        std::string message
 +            = formatString("Expression '%s' is invalid in this context",
 +                           text.c_str());
 +        GMX_THROW(InvalidInputError(message));
      }
  
      child->flags   |= SEL_ALLOCVAL;
      *param->nvalptr = param->val.nr;
      /* Rest of the initialization is done during compilation in
       * init_method(). */
 -
 -    return true;
  }
  
  /*! \brief
   * as the value \p i of \p param.
   * This function is used internally by parse_values_std().
   */
 -static bool
 +static void
  set_expr_value_store(const SelectionTreeElementPointer &sel,
                       gmx_ana_selparam_t *param, int i, void *scanner)
  {
      if (sel->v.type != GROUP_VALUE && !(sel->flags & SEL_SINGLEVAL))
      {
 -        _gmx_selparser_error(scanner, "invalid expression value");
 -        return false;
 +        std::string text(_gmx_sel_lexer_get_text(scanner, sel->location()));
 +        std::string message
 +            = formatString("Expression '%s' is invalid in this context",
 +                           text.c_str());
 +        GMX_THROW(InvalidInputError(message));
      }
      switch (sel->v.type)
      {
          case POS_VALUE:   sel->v.u.p = &param->val.u.p[i]; break;
          case GROUP_VALUE: sel->v.u.g = &param->val.u.g[i]; break;
          default: /* Error */
 -            GMX_THROW(gmx::InternalError("Invalid value type"));
 +            GMX_THROW(InternalError("Invalid value type"));
      }
      sel->v.nr     = 1;
      sel->v.nalloc = -1;
 -    return true;
  }
  
  /*! \brief
   * \param     param  Parameter to parse.
   * \param     root   Selection element to which child expressions are added.
   * \param[in] scanner Scanner data structure.
 - * \returns   true if the values were parsed successfully, false otherwise.
   *
   * For integer ranges, the sequence of numbers from the first to second value
   * is stored, each as a separate value.
   */
 -static bool
 +static void
  parse_values_std(const SelectionParserValueList &values,
                   gmx_ana_selparam_t *param,
                   const SelectionTreeElementPointer &root, void *scanner)
      {
          if (values.size() > 1)
          {
 -            _gmx_selparser_error(scanner, "more than one value not supported");
 -            return false;
 +            GMX_THROW(InvalidInputError(
 +                              "Only a single value or a single expression is "
 +                              "supported in this context"));
          }
          if (values.front().hasExpressionValue())
          {
              SelectionTreeElementPointer child
 -                = add_child(root, param, values.front().expr, scanner);
 -            if (!child)
 -            {
 -                return false;
 -            }
 +                          = add_child(root, param, values.front().expr, scanner);
              child->flags |= SEL_ALLOCVAL;
              if (child->v.type != GROUP_VALUE && (child->flags & SEL_ATOMVAL))
              {
                  {
                      *param->nvalptr = -1;
                  }
 -                return true;
 +                return;
              }
              param->flags  &= ~SPAR_ATOMVAL;
              param->val.nr  = 1;
              {
                  _gmx_selvalue_reserve(&param->val, 1);
              }
 -            return set_expr_value_store(child, param, 0, scanner);
 +            set_expr_value_store(child, param, 0, scanner);
 +            return;
          }
          /* If we reach here, proceed with normal parameter handling */
          param->val.nr = 1;
      SelectionParserValueList::const_iterator value;
      for (value = values.begin(); value != values.end() && i < param->val.nr; ++value)
      {
 -        if (value->type != param->val.type)
 -        {
 -            _gmx_selparser_error(scanner, "incorrect value skipped");
 -            continue;
 -        }
 +        GMX_RELEASE_ASSERT(value->type == param->val.type,
 +                           "Invalid value type (should have been caught earlier)");
          if (value->hasExpressionValue())
          {
              SelectionTreeElementPointer child
                  = add_child(root, param, value->expr, scanner);
 -            /* Check that the expression is valid */
 -            if (!child)
 -            {
 -                return false;
 -            }
 -            if (!set_expr_value_store(child, param, i, scanner))
 -            {
 -                return false;
 -            }
 +            set_expr_value_store(child, param, i, scanner);
              if (child->flags & SEL_DYNAMIC)
              {
                  bDynamic = true;
              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
                      {
                          {
                              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)
                      {
 -                        _gmx_selparser_error(scanner, "real ranges not supported");
 -                        return false;
 +                        std::string text(_gmx_sel_lexer_get_text(scanner, value->location()));
 +                        std::string message
 +                            = formatString("Real range ('%s') is not supported in this context",
 +                                           text.c_str());
 +                        GMX_THROW(InvalidInputError(message));
                      }
                      param->val.u.r[i] = value->u.r.r1;
                      break;
                  case STR_VALUE:
 -                    param->val.u.s[i] = strdup(value->stringValue().c_str());
 +                    param->val.u.s[i] = gmx_strdup(value->stringValue().c_str());
                      break;
                  case POS_VALUE:
                      gmx_ana_pos_init_const(&param->val.u.p[i], value->u.x);
                      break;
                  case NO_VALUE:
                  case GROUP_VALUE:
 -                    GMX_THROW(gmx::InternalError("Invalid non-expression value type"));
 +                    GMX_THROW(InternalError("Invalid non-expression value type"));
              }
          }
          ++i;
      }
      if (value != values.end())
      {
 -        _gmx_selparser_error(scanner, "extra values");
 -        return false;
 +        std::string message
 +            = formatString("Too many values provided, expected %d",
 +                           param->val.nr);
 +        GMX_THROW(InvalidInputError(message));
      }
      if (i < param->val.nr)
      {
 -        _gmx_selparser_error(scanner, "not enough values");
 -        return false;
 +        std::string message
 +            = formatString("Too few values provided, expected %d",
 +                           param->val.nr);
 +        GMX_THROW(InvalidInputError(message));
      }
      if (!bDynamic)
      {
          *param->nvalptr = param->val.nr;
      }
      param->nvalptr = NULL;
 -
 -    return true;
  }
  
  /*! \brief
   * \param[in] values List of values.
   * \param     param  Parameter to parse.
   * \param[in] scanner Scanner data structure.
 - * \returns   true if the values were parsed successfully, false otherwise.
   */
 -static bool
 +static void
  parse_values_bool(const std::string &name,
                    const SelectionParserValueList &values,
                    gmx_ana_selparam_t *param, void *scanner)
  {
 +    GMX_UNUSED_VALUE(scanner);
      GMX_ASSERT(param->val.type == NO_VALUE,
                 "Boolean parser called for non-boolean parameter");
      if (values.size() > 1 || (!values.empty() && values.front().type != INT_VALUE))
      {
 -        _gmx_selparser_error(scanner, "parameter takes only a yes/no/on/off/0/1 value");
 -        return false;
 +        std::string message
 +            = formatString("'%s' only accepts yes/no/on/off/0/1 (and empty) as a value",
 +                           param->name);
 +        GMX_THROW(InvalidInputError(message));
      }
  
      bool bSetNo = false;
      }
      if (bSetNo && !values.empty())
      {
 -        _gmx_selparser_error(scanner, "parameter 'no%s' should not have a value",
 -                             param->name);
 -        return false;
 +        std::string message
 +            = formatString("'no%s' cannot be followed by any value",
 +                           param->name);
 +        GMX_THROW(InvalidInputError(message));
      }
      if (!values.empty() && values.front().u.i.i1 == 0)
      {
      }
  
      *param->val.u.b = bSetNo ? false : true;
 -    return true;
  }
  
  /*! \brief
   * \param[in] scanner Scanner data structure.
   * \returns   true if the values were parsed successfully, false otherwise.
   */
 -static bool
 +static void
  parse_values_enum(const SelectionParserValueList &values,
                    gmx_ana_selparam_t             *param,
                    void                           *scanner)
                 "Enum parser called for non-string parameter");
      if (values.size() != 1)
      {
 -        _gmx_selparser_error(scanner, "a single value is required");
 -        return false;
 +        GMX_THROW(InvalidInputError(
 +                          "Only a single string value is supported in this context"));
      }
      const SelectionParserValue &value = values.front();
      GMX_RELEASE_ASSERT(value.type == param->val.type,
                         "Invalid value type (should have been caught earlier)");
      if (value.hasExpressionValue())
      {
 -        _gmx_selparser_error(scanner, "expression value for enumerated parameter not supported");
 -        return false;
 +        std::string text(_gmx_sel_lexer_get_text(scanner, value.location()));
 +        std::string message
 +            = formatString("Expression ('%s') is not supported in this context",
 +                           text.c_str());
 +        GMX_THROW(InvalidInputError(message));
      }
  
      const std::string &svalue = value.stringValue();
      int                match  = 0;
      while (param->val.u.s[i] != NULL)
      {
 -        if (gmx::startsWith(param->val.u.s[i], svalue))
 +        if (startsWith(param->val.u.s[i], svalue))
          {
              /* Check if there is a duplicate match */
              if (match > 0)
              {
 -                _gmx_selparser_error(scanner, "ambiguous value");
 -                return false;
 +                std::string message
 +                    = formatString("Value '%s' is ambiguous", svalue.c_str());
 +                GMX_THROW(InvalidInputError(message));
              }
              match = i;
          }
      }
      if (match == 0)
      {
 -        _gmx_selparser_error(scanner, "invalid value");
 -        return false;
 +        std::string message
 +            = formatString("Value '%s' is not recognized", svalue.c_str());
 +        GMX_THROW(InvalidInputError(message));
      }
      param->val.u.s[0] = param->val.u.s[match];
 -    return true;
  }
  
  /*! \brief
@@@ -1035,25 -1013,24 +1035,25 @@@ convert_const_values(SelectionParserVal
          if (value->hasExpressionValue() && value->expr->v.type != GROUP_VALUE &&
              value->expr->type == SEL_CONST)
          {
 -            SelectionTreeElementPointer expr = value->expr;
 +            SelectionTreeElementPointer expr     = value->expr;
 +            const SelectionLocation    &location = value->location();
              switch (expr->v.type)
              {
                  case INT_VALUE:
 -                    *value = SelectionParserValue::createInteger(expr->v.u.i[0]);
 +                    *value = SelectionParserValue::createInteger(expr->v.u.i[0], location);
                      break;
                  case REAL_VALUE:
 -                    *value = SelectionParserValue::createReal(expr->v.u.r[0]);
 +                    *value = SelectionParserValue::createReal(expr->v.u.r[0], location);
                      break;
                  case STR_VALUE:
 -                    *value = SelectionParserValue::createString(expr->v.u.s[0]);
 +                    *value = SelectionParserValue::createString(expr->v.u.s[0], location);
                      break;
                  case POS_VALUE:
 -                    *value = SelectionParserValue::createPosition(expr->v.u.p->x[0]);
 +                    *value = SelectionParserValue::createPosition(expr->v.u.p->x[0], location);
                      break;
                  default:
 -                    GMX_THROW(gmx::InternalError(
 -                                      "Unsupported constant expression value type"));
 +                    GMX_RELEASE_ASSERT(false,
 +                                       "Unsupported constant expression value type");
              }
          }
      }
   * \param     params  Array of parameters to parse.
   * \param     root    Selection element to which child expressions are added.
   * \param[in] scanner Scanner data structure.
 - * \returns   true if the parameters were parsed successfully, false otherwise.
   *
   * Initializes the \p params array based on the parameters in \p pparams.
   * See the documentation of \c gmx_ana_selparam_t for different options
   * The list \p pparams and any associated values are freed after the parameters
   * have been processed, no matter is there was an error or not.
   */
 -bool
 +void
  _gmx_sel_parse_params(const gmx::SelectionParserParameterList &pparams,
                        int nparam, gmx_ana_selparam_t *params,
                        const gmx::SelectionTreeElementPointer &root,
                        void *scanner)
  {
 -    gmx::MessageStringCollector *errors = _gmx_sel_lexer_error_reporter(scanner);
 -    gmx_ana_selparam_t          *oparam;
 -    bool                         bOk, rc;
 -    int                          i;
 -
 +    ExceptionInitializer errors("");
      /* Check that the value pointers of SPAR_VARNUM parameters are NULL and
       * that they are not NULL for other parameters */
 -    bOk = true;
 -    for (i = 0; i < nparam; ++i)
 +    for (int i = 0; i < nparam; ++i)
      {
 -        std::string                contextStr = gmx::formatString("In parameter '%s'", params[i].name);
 -        gmx::MessageStringContext  context(errors, contextStr);
 -        if (params[i].val.type != POS_VALUE && (params[i].flags & (SPAR_VARNUM | SPAR_ATOMVAL)))
 +        if (params[i].val.type != POS_VALUE
 +            && (params[i].flags & (SPAR_VARNUM | SPAR_ATOMVAL)))
          {
 -            if (params[i].val.u.ptr != NULL)
 -            {
 -                _gmx_selparser_error(scanner, "value pointer is not NULL "
 -                                     "although it should be for SPAR_VARNUM "
 -                                     "and SPAR_ATOMVAL parameters");
 -            }
 -            if ((params[i].flags & SPAR_VARNUM)
 -                && (params[i].flags & SPAR_DYNAMIC) && !params[i].nvalptr)
 -            {
 -                _gmx_selparser_error(scanner, "nvalptr is NULL but both "
 -                                     "SPAR_VARNUM and SPAR_DYNAMIC are specified");
 -                bOk = false;
 -            }
 +            GMX_RELEASE_ASSERT(params[i].val.u.ptr == NULL,
 +                               "value pointer is not NULL "
 +                               "although it should be for SPAR_VARNUM "
 +                               "and SPAR_ATOMVAL parameters");
 +            GMX_RELEASE_ASSERT(!((params[i].flags & SPAR_VARNUM)
 +                                 && (params[i].flags & SPAR_DYNAMIC))
 +                               || params[i].nvalptr != NULL,
 +                               "nvalptr is NULL but both "
 +                               "SPAR_VARNUM and SPAR_DYNAMIC are specified");
          }
          else
          {
 -            if (params[i].val.u.ptr == NULL)
 -            {
 -                _gmx_selparser_error(scanner, "value pointer is NULL");
 -                bOk = false;
 -            }
 +            GMX_RELEASE_ASSERT(params[i].val.u.ptr != NULL,
 +                               "value pointer is NULL");
          }
      }
 -    if (!bOk)
 -    {
 -        return false;
 -    }
      /* Parse the parameters */
 -    i = 0;
 +    int nullParamIndex = 0;
      SelectionParserParameterList::const_iterator pparam;
      for (pparam = pparams.begin(); pparam != pparams.end(); ++pparam)
      {
 -        std::string contextStr;
 -        /* Find the parameter and make some checks */
 -        if (!pparam->name().empty())
 -        {
 -            contextStr = gmx::formatString("In parameter '%s'", pparam->name().c_str());
 -            i          = -1;
 -            oparam     = gmx_ana_selparam_find(pparam->name().c_str(), nparam, params);
 -        }
 -        else if (i >= 0)
 +        try
          {
 -            contextStr = gmx::formatString("In value %d", i + 1);
 -            oparam     = &params[i];
 -            if (oparam->name != NULL)
 +            // Always assigned afterwards, but cppcheck does not see that.
 +            gmx_ana_selparam_t *oparam = NULL;
 +            /* Find the parameter and make some checks */
 +            if (!pparam->name().empty())
              {
 -                oparam = NULL;
 -                _gmx_selparser_error(scanner, "too many NULL parameters provided");
 -                bOk = false;
 -                continue;
 +                nullParamIndex = -1;
 +                oparam
 +                    = gmx_ana_selparam_find(pparam->name().c_str(), nparam, params);
 +                GMX_RELEASE_ASSERT(oparam != NULL, "Inconsistent selection parameter");
              }
 -            ++i;
 -        }
 -        else
 -        {
 -            _gmx_selparser_error(scanner, "all NULL parameters should appear in the beginning of the list");
 -            bOk = false;
 -            continue;
 -        }
 -        gmx::MessageStringContext  context(errors, contextStr);
 -        if (!oparam)
 -        {
 -            _gmx_selparser_error(scanner, "unknown parameter skipped");
 -            bOk = false;
 -            continue;
 -        }
 -        if (oparam->val.type != NO_VALUE && pparam->values().empty())
 -        {
 -            _gmx_selparser_error(scanner, "no value provided");
 -            bOk = false;
 -            continue;
 -        }
 -        if (oparam->flags & SPAR_SET)
 -        {
 -            _gmx_selparser_error(scanner, "parameter set multiple times, extra values skipped");
 -            bOk = false;
 -            continue;
 -        }
 -        oparam->flags |= SPAR_SET;
 -        /* Process the values for the parameter */
 -        convert_const_values(pparam->values_.get());
 -        if (convert_values(pparam->values_.get(), oparam->val.type, scanner) != 0)
 -        {
 -            _gmx_selparser_error(scanner, "invalid value");
 -            bOk = false;
 -            continue;
 -        }
 -        if (oparam->val.type == NO_VALUE)
 -        {
 -            rc = parse_values_bool(pparam->name(), pparam->values(), oparam, scanner);
 -        }
 -        else if (oparam->flags & SPAR_RANGES)
 -        {
 -            rc = parse_values_range(pparam->values(), oparam, scanner);
 -        }
 -        else if (oparam->flags & SPAR_VARNUM)
 -        {
 -            if (pparam->values().size() == 1
 -                && pparam->values().front().hasExpressionValue())
 +            else if (nullParamIndex >= 0)
 +            {
 +                oparam = &params[nullParamIndex];
 +                if (oparam->name != NULL)
 +                {
 +                    std::string text(_gmx_sel_lexer_get_text(scanner, pparam->location()));
 +                    std::string message
 +                        = formatString("Unexpected '%s'", text.c_str());
 +                    GMX_THROW(InvalidInputError(message));
 +                }
 +                ++nullParamIndex;
 +            }
 +            else
 +            {
 +                GMX_RELEASE_ASSERT(false, "All NULL parameters should appear in "
 +                                   "the beginning of the list");
 +            }
 +            if (oparam->flags & SPAR_SET)
 +            {
 +                std::string message
 +                    = formatString("'%s' appears multiple times",
 +                                   pparam->name().c_str());
 +                GMX_THROW(InvalidInputError(message));
 +            }
 +            oparam->flags |= SPAR_SET;
 +            if (oparam->val.type != NO_VALUE && pparam->values().empty())
 +            {
 +                std::string text;
 +                if (pparam->name().empty())
 +                {
 +                    text = root->name();
 +                }
 +                else
 +                {
 +                    text = _gmx_sel_lexer_get_text(scanner, pparam->location());
 +                }
 +                std::string message
 +                    = formatString("'%s' should be followed by a value/expression",
 +                                   text.c_str());
 +                GMX_THROW(InvalidInputError(message));
 +            }
 +            /* Process the values for the parameter */
 +            convert_const_values(pparam->values_.get());
 +            convert_values(pparam->values_.get(), oparam->val.type, scanner);
 +            if (oparam->val.type == NO_VALUE)
 +            {
 +                parse_values_bool(pparam->name(), pparam->values(), oparam, scanner);
 +            }
 +            else if (oparam->flags & SPAR_RANGES)
 +            {
 +                parse_values_range(pparam->values(), oparam, scanner);
 +            }
 +            else if (oparam->flags & SPAR_VARNUM)
 +            {
 +                if (pparam->values().size() == 1
 +                    && pparam->values().front().hasExpressionValue())
 +                {
 +                    parse_values_varnum_expr(pparam->values(), oparam, root, scanner);
 +                }
 +                else
 +                {
 +                    parse_values_varnum(pparam->values(), oparam, root, scanner);
 +                }
 +            }
 +            else if (oparam->flags & SPAR_ENUMVAL)
              {
 -                rc = parse_values_varnum_expr(pparam->values(), oparam, root, scanner);
 +                parse_values_enum(pparam->values(), oparam, scanner);
              }
              else
              {
 -                rc = parse_values_varnum(pparam->values(), oparam, root, scanner);
 +                parse_values_std(pparam->values(), oparam, root, scanner);
              }
          }
 -        else if (oparam->flags & SPAR_ENUMVAL)
 -        {
 -            rc = parse_values_enum(pparam->values(), oparam, scanner);
 -        }
 -        else
 -        {
 -            rc = parse_values_std(pparam->values(), oparam, root, scanner);
 -        }
 -        if (!rc)
 +        catch (UserInputError &ex)
          {
 -            bOk = false;
 +            if (!pparam->name().empty())
 +            {
 +                std::string text(_gmx_sel_lexer_get_text(scanner, pparam->location()));
 +                ex.prependContext(formatString("In '%s'", text.c_str()));
 +            }
 +            errors.addCurrentExceptionAsNested();
          }
      }
      /* Check that all required parameters are present */
 -    for (i = 0; i < nparam; ++i)
 +    for (int i = 0; i < nparam; ++i)
      {
          if (!(params[i].flags & SPAR_OPTIONAL) && !(params[i].flags & SPAR_SET))
          {
 -            _gmx_selparser_error(scanner, "required parameter '%s' not specified", params[i].name);
 -            bOk = false;
 +            std::string message;
 +            if (params[i].name == NULL)
 +            {
 +                message = formatString("'%s' should be followed by a value/expression",
 +                                       root->name().c_str());
 +            }
 +            else
 +            {
 +                message = formatString("'%s' is missing", params[i].name);
 +            }
 +            InvalidInputError ex(message);
 +            errors.addNested(ex);
          }
      }
 -
 -    return bOk;
 +    if (errors.hasNestedExceptions())
 +    {
 +        GMX_THROW(InvalidInputError(errors));
 +    }
  }
index f97978f9885294cf6dc02e8e401e285a4f16520e,cef245c5d928991e510e7436cefdbd856aa16fd7..01450da98f516905f7ecd4ebc0743bcb63d5b1f6
@@@ -1,7 -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.
   * \author Teemu Murtola <teemu.murtola@gmail.com>
   * \ingroup module_selection
   */
 +#include "gmxpre.h"
 +
  #include <stdlib.h>
  
  #include "gromacs/legacyheaders/macros.h"
 -
 -#include "gromacs/selection/selmethod.h"
  #include "gromacs/utility/exceptions.h"
  #include "gromacs/utility/smalloc.h"
  
  #include "keywords.h"
  #include "parsetree.h"
  #include "selelem.h"
 +#include "selmethod.h"
  
  /*! \internal
   * \brief
@@@ -231,11 -230,12 +231,11 @@@ init_data_same(int /* npar */, gmx_ana_
   * \param[in,out] method  The method to initialize.
   * \param[in,out] params  Pointer to the first parameter.
   * \param[in]     scanner Scanner data structure.
 - * \returns       0 on success, a non-zero error code on error.
   *
 - * If \p *method is not a \c same method, this function returns zero
 + * If \p *method is not a \c same method, this function returns
   * immediately.
   */
 -int
 +void
  _gmx_selelem_custom_init_same(gmx_ana_selmethod_t                           **method,
                                const gmx::SelectionParserParameterListPointer &params,
                                void                                           *scanner)
      /* Do nothing if this is not a same method. */
      if (!*method || (*method)->name != sm_same.name || params->empty())
      {
 -        return 0;
 +        return;
      }
  
      const gmx::SelectionParserValueList &kwvalues = params->front().values();
      if (kwvalues.size() != 1 || !kwvalues.front().hasExpressionValue()
          || kwvalues.front().expr->type != SEL_EXPRESSION)
      {
 -        _gmx_selparser_error(scanner, "'same' should be followed by a single keyword");
 -        return -1;
 +        GMX_THROW(gmx::InvalidInputError("'same' should be followed by a single keyword"));
      }
      gmx_ana_selmethod_t *kwmethod = kwvalues.front().expr->u.expr.method;
      if (kwmethod->type == STR_VALUE)
      gmx::SelectionParserParameterList::iterator asparam = ++params->begin();
      if (asparam != params->end() && asparam->name() == sm_same.param[1].name)
      {
 -        gmx::SelectionParserParameterList    kwparams;
 -        gmx::SelectionParserValueListPointer values(
 -                new gmx::SelectionParserValueList(asparam->values()));
 -        kwparams.push_back(
 -                gmx::SelectionParserParameter::create(NULL, move(values)));
 -
 +        const gmx::SelectionParserValueList &asvalues = asparam->values();
 +        if (asvalues.size() != 1 || !asvalues.front().hasExpressionValue())
 +        {
 +            // TODO: Think about providing more informative context.
 +            GMX_THROW(gmx::InvalidInputError("'same ... as' should be followed by a single expression"));
 +        }
 +        const gmx::SelectionTreeElementPointer &child = asvalues.front().expr;
          /* Create a second keyword evaluation element for the keyword given as
           * the first parameter, evaluating the keyword in the group given by the
           * second parameter. */
          gmx::SelectionTreeElementPointer kwelem
 -            = _gmx_sel_init_keyword_evaluator(kwmethod, kwparams, scanner);
 -        // FIXME: Use exceptions.
 -        if (!kwelem)
 -        {
 -            return -1;
 -        }
 +            = _gmx_sel_init_keyword_evaluator(kwmethod, child, scanner);
          /* Replace the second parameter with one with a value from \p kwelem. */
          std::string pname = asparam->name();
          *asparam = gmx::SelectionParserParameter::createFromExpression(pname, kwelem);
      }
 -    return 0;
  }
  
  static void
@@@ -337,6 -343,10 +337,10 @@@ init_frame_same_int(t_topology * /* to
  
      /* Collapse adjacent values, and check whether the array is sorted. */
      d->bSorted = true;
+     if (d->nas == 0)
+     {
+         return;
+     }
      for (i = 1, j = 0; i < d->nas; ++i)
      {
          if (d->as.i[i] != d->as.i[j])
@@@ -463,6 -473,10 +467,10 @@@ init_frame_same_str(t_topology * /* to
       * For strings, it's unlikely that the values would be sorted originally,
       * so set bSorted always to false. */
      d->bSorted        = false;
+     if (d->nas == 0)
+     {
+         return;
+     }
      d->as_s_sorted[0] = d->as.s[0];
      for (i = 1, j = 0; i < d->nas; ++i)
      {
index f17c17d2b3143ebe4b6bb9af1a9741ff65d454bc,6a40ad7462b8411f47554e27e0a9c69022b0d634..8521a27d444767bb0e6bedd6768cb07f3564174e
   * \author Teemu Murtola <teemu.murtola@gmail.com>
   * \ingroup module_selection
   */
 +#include "gmxpre.h"
 +
 +#include "gromacs/selection/selectioncollection.h"
 +
  #include <gtest/gtest.h>
  
 +#include "gromacs/fileio/trx.h"
  #include "gromacs/options/basicoptions.h"
  #include "gromacs/options/options.h"
  #include "gromacs/selection/indexutil.h"
 -#include "gromacs/selection/selectioncollection.h"
  #include "gromacs/selection/selection.h"
 +#include "gromacs/topology/topology.h"
  #include "gromacs/utility/arrayref.h"
  #include "gromacs/utility/exceptions.h"
  #include "gromacs/utility/flags.h"
@@@ -523,27 -518,6 +523,27 @@@ TEST_F(SelectionCollectionTest, Handles
      // EXPECT_THROW_GMX(sc_.compile(), gmx::APIError);
  }
  
 +TEST_F(SelectionCollectionTest, HandlesOutOfRangeAtomIndexInGroup)
 +{
 +    ASSERT_NO_THROW_GMX(sc_.setTopology(NULL, 5));
 +    ASSERT_NO_THROW_GMX(loadIndexGroups("simple.ndx"));
 +    EXPECT_THROW_GMX(sc_.parseFromString("group \"GrpB\""), gmx::InconsistentInputError);
 +}
 +
 +TEST_F(SelectionCollectionTest, HandlesOutOfRangeAtomIndexInGroupDelayed)
 +{
 +    ASSERT_NO_THROW_GMX(loadIndexGroups("simple.ndx"));
 +    ASSERT_NO_THROW_GMX(sc_.parseFromString("group \"GrpB\""));
 +    EXPECT_THROW_GMX(sc_.setTopology(NULL, 5), gmx::InconsistentInputError);
 +}
 +
 +TEST_F(SelectionCollectionTest, HandlesOutOfRangeAtomIndexInGroupDelayed2)
 +{
 +    ASSERT_NO_THROW_GMX(sc_.setTopology(NULL, 5));
 +    ASSERT_NO_THROW_GMX(sc_.parseFromString("group \"GrpB\""));
 +    EXPECT_THROW_GMX(loadIndexGroups("simple.ndx"), gmx::InconsistentInputError);
 +}
 +
  TEST_F(SelectionCollectionTest, RecoversFromMissingMoleculeInfo)
  {
      ASSERT_NO_THROW_GMX(sc_.parseFromString("molindex 1 to 5"));
@@@ -587,35 -561,7 +587,35 @@@ TEST_F(SelectionCollectionTest, Recover
      EXPECT_THROW_GMX(sc_.evaluate(frame_, NULL), gmx::InconsistentInputError);
  }
  
 -// TODO: Tests for evaluation errors
 +TEST_F(SelectionCollectionTest, HandlesFramesWithTooSmallAtomSubsets)
 +{
 +    ASSERT_NO_THROW_GMX(sc_.parseFromString("atomnr 3 to 10"));
 +    ASSERT_NO_FATAL_FAILURE(loadTopology("simple.gro"));
 +    ASSERT_NO_THROW_GMX(sc_.compile());
 +    frame_->natoms = 8;
 +    EXPECT_THROW_GMX(sc_.evaluate(frame_, NULL), gmx::InconsistentInputError);
 +}
 +
 +TEST_F(SelectionCollectionTest, HandlesFramesWithTooSmallAtomSubsets2)
 +{
 +    // Evaluating the positions will require atoms 1-9.
 +    ASSERT_NO_THROW_GMX(sc_.parseFromString("whole_res_com of atomnr 2 5 7"));
 +    ASSERT_NO_FATAL_FAILURE(loadTopology("simple.gro"));
 +    ASSERT_NO_THROW_GMX(sc_.compile());
 +    frame_->natoms = 8;
 +    EXPECT_THROW_GMX(sc_.evaluate(frame_, NULL), gmx::InconsistentInputError);
 +}
 +
 +TEST_F(SelectionCollectionTest, HandlesFramesWithTooSmallAtomSubsets3)
 +{
 +    ASSERT_NO_THROW_GMX(sc_.parseFromString("mindistance from atomnr 1 to 5 < 2"));
 +    ASSERT_NO_FATAL_FAILURE(loadTopology("simple.gro"));
 +    ASSERT_NO_THROW_GMX(sc_.compile());
 +    frame_->natoms = 10;
 +    EXPECT_THROW_GMX(sc_.evaluate(frame_, NULL), gmx::InconsistentInputError);
 +}
 +
 +// TODO: Tests for more evaluation errors
  
  
  /********************************************************************
@@@ -1028,25 -974,12 +1028,25 @@@ TEST_F(SelectionCollectionDataTest, Han
      ASSERT_NO_FATAL_FAILURE(runCompiler());
  }
  
 +
  TEST_F(SelectionCollectionDataTest, HandlesConstantPositions)
  {
      static const char * const selections[] = {
          "[1, -2, 3.5]"
      };
 -    setFlags(TestFlags() | efTestEvaluation | efTestPositionCoordinates);
 +    setFlags(TestFlags() | efTestEvaluation | efTestPositionCoordinates
 +             | efTestPositionMapping);
 +    runTest("simple.gro", selections);
 +}
 +
 +
 +TEST_F(SelectionCollectionDataTest, HandlesConstantPositionsWithModifiers)
 +{
 +    static const char * const selections[] = {
 +        "[0, 0, 0] plus [0, 1, 0]"
 +    };
 +    setFlags(TestFlags() | efTestEvaluation | efTestPositionCoordinates
 +             | efTestPositionMapping);
      runTest("simple.gro", selections);
  }
  
@@@ -1061,6 -994,32 +1061,32 @@@ TEST_F(SelectionCollectionDataTest, Han
  }
  
  
+ TEST_F(SelectionCollectionDataTest, HandlesOverlappingIntegerRanges)
+ {
+     static const char * const selections[] = {
+         "atomnr 2 to 4 5 to 8",
+         "atomnr 2 to 5 4 to 7"
+     };
+     ASSERT_NO_FATAL_FAILURE(runTest(10, selections));
+ }
+ TEST_F(SelectionCollectionDataTest, HandlesOverlappingRealRanges)
+ {
+     static const char * const selections[] = {
+         "charge {-0.35 to -0.05 0.25 to 0.75}",
+         "charge {0.05 to -0.3 -0.05 to 0.55}"
+     };
+     ASSERT_NO_FATAL_FAILURE(runParser(selections));
+     ASSERT_NO_FATAL_FAILURE(loadTopology("simple.gro"));
+     for (int i = 0; i < top_->atoms.nr; ++i)
+     {
+         top_->atoms.atom[i].q = i / 10.0 - 0.5;
+     }
+     ASSERT_NO_FATAL_FAILURE(runCompiler());
+ }
  TEST_F(SelectionCollectionDataTest, HandlesForcedStringMatchingMode)
  {
      static const char * const selections[] = {
@@@ -1128,47 -1087,16 +1154,57 @@@ TEST_F(SelectionCollectionDataTest, Han
  }
  
  
+ TEST_F(SelectionCollectionDataTest, HandlesEmptyReferenceForSame)
+ {
+     static const char * const selections[] = {
+         "same residue as none",
+         "same resname as none"
+     };
+     runTest("simple.gro", selections);
+ }
 +TEST_F(SelectionCollectionDataTest, HandlesPositionModifiersForKeywords)
 +{
 +    static const char * const selections[] = {
 +        "res_cog x > 2",
 +        "name CB and res_cog y > 2.5"
 +    };
 +    setFlags(TestFlags() | efTestEvaluation);
 +    runTest("simple.gro", selections);
 +}
 +
 +
 +TEST_F(SelectionCollectionDataTest, HandlesPositionModifiersForMethods)
 +{
 +    static const char * const selections[] = {
 +        "res_cog distance from cog of resnr 1 < 2",
 +        "res_cog within 2 of cog of resnr 1"
 +    };
 +    setFlags(TestFlags() | efTestEvaluation);
 +    runTest("simple.gro", selections);
 +}
 +
 +
 +TEST_F(SelectionCollectionDataTest, HandlesKeywordOfPositions)
 +{
 +    static const char * const selections[] = {
 +        "x < y of cog of resnr 2"
 +    };
 +    setFlags(TestFlags() | efTestEvaluation);
 +    runTest("simple.gro", selections);
 +}
 +
 +TEST_F(SelectionCollectionDataTest, HandlesKeywordOfPositionsInArithmetic)
 +{
 +    static const char * const selections[] = {
 +        "x - y of cog of resnr 2 < 0"
 +    };
 +    setFlags(TestFlags() | efTestEvaluation);
 +    runTest("simple.gro", selections);
 +}
 +
 +
  TEST_F(SelectionCollectionDataTest, HandlesNumericComparisons)
  {
      static const char * const selections[] = {