Fix internal error with selection "resname help".

- Always recognize "help" as a help request token, which then causes a
  syntax error in invalid context instead of an internal error in the
  tokenizer symbol table lookup.
- Change the internal error in question to use an exception for error
  reporting to make it possible to easily check it in a unit test.

Change-Id: I4a4057840b47a818cc840654945e84ec536e35b0

diff --git a/src/gromacs/selection/scanner.cpp b/src/gromacs/selection/scanner.cpp
index 451e8d058825b437941456e8359092c7a3211fde..1c1bc10dc27ae47219d89a5baf83f6c80ce7eb75 100644 (file)
--- a/src/gromacs/selection/scanner.cpp
+++ b/src/gromacs/selection/scanner.cpp
@@ -361,12 +361,12 @@ static yyconst flex_int16_t yy_accept[89] =
         0,    0,    0,    0,    0,    0,    0,    0,    0,    0,
        27,   25,   23,    6,   20,   25,    1,   25,   25,    2,
         6,   21,   25,   22,   25,   24,   22,   22,   22,   22,
-       22,   22,   25,   22,   22,   22,   22,   22,   11,    8,
+       22,   22,   22,   25,   22,   22,   22,   22,   11,    8,
        10,   10,    9,   23,   21,    0,    4,    0,    1,   17,
         3,    3,    2,   24,   24,   22,    5,   22,   22,   22,
-       18,   15,   22,   18,   16,   13,   22,   12,   22,   22,
-        8,    9,    0,    0,    3,   17,   22,   20,   19,   13,
-       22,    0,    3,    3,   22,    7,   14,    0
+       22,   18,   15,   22,   18,   16,   13,   22,   12,   22,
+        8,    9,    0,    0,    3,   17,   22,   22,   20,   19,
+       13,    0,    3,    3,   22,    7,   14,    0
     } ;
 
 static yyconst flex_int32_t yy_ec[256] =
@@ -411,15 +411,15 @@ static yyconst flex_int32_t yy_meta[35] =
 
 static yyconst flex_int16_t yy_base[94] =
     {   0,
-        0,    0,  131,  130,   10,   12,  132,  131,   45,    0,
-      153,  158,  150,  158,  138,   75,    0,  143,  139,   72,
-      158,  135,  134,    0,  143,  136,  119,  115,  116,  113,
-      114,  113,  104,   62,  111,   68,  116,  115,  158,  132,
-      158,  158,    0,  131,  158,   79,  158,  127,    0,  158,
-       84,   87,   91,  122,   31,    0,  158,  111,  103,   98,
-        0,    0,   99,  158,    0,   96,  104,    0,   95,   99,
-      120,    0,   34,  107,   76,    0,   82,    0,    0,    0,
-       83,   99,   98,   95,   76,    0,    0,  158,  111,  115,
+        0,    0,  129,  128,   10,   12,    0,    0,   45,    0,
+      153,  156,  150,  156,  138,   75,    0,  143,  139,   72,
+      156,  135,  134,    0,  143,  136,  119,  115,  122,  115,
+      112,  113,  112,  103,   62,  110,   68,  115,  156,  132,
+      156,  156,    0,  131,  156,   79,  156,  127,    0,  156,
+       84,   87,   91,  122,   31,    0,  156,  111,  103,  104,
+       97,    0,    0,   98,  156,    0,   95,  103,    0,   94,
+      120,    0,   34,  107,   76,    0,   82,   83,    0,    0,
+        0,   99,   98,   95,   76,    0,    0,  156,  111,  115,
       117,   94,   84
 
     } ;
@@ -429,43 +429,41 @@ static yyconst flex_int16_t yy_def[94] =
        88,    1,    1,    1,    1,    1,    1,    1,   88,    9,
        88,   88,   88,   88,   88,   89,   90,   88,   88,   91,
        88,   88,   88,   92,   88,   91,   92,   92,   92,   92,
-       92,   92,   88,   92,   92,   92,   92,   92,   88,   88,
+       92,   92,   92,   88,   92,   92,   92,   92,   88,   88,
        88,   88,   93,   88,   88,   89,   88,   88,   90,   88,
        88,   88,   91,   91,   91,   92,   88,   92,   92,   92,
-       92,   92,   92,   88,   92,   92,   92,   92,   92,   92,
+       92,   92,   92,   92,   88,   92,   92,   92,   92,   92,
        88,   93,   88,   88,   91,   92,   92,   92,   92,   92,
        92,   88,   88,   88,   92,   92,   92,    0,   88,   88,
        88,   88,   88
 
     } ;
 
-static yyconst flex_int16_t yy_nxt[193] =
+static yyconst flex_int16_t yy_nxt[191] =
     {   0,
        12,   13,   14,   15,   16,   17,   18,   12,   19,   20,
        21,   22,   23,   24,   24,   25,   26,   27,   24,   24,
-       24,   28,   24,   24,   29,   30,   24,   24,   24,   31,
-       24,   32,   24,   33,   35,   36,   35,   36,   74,   88,
-       75,   82,   37,   83,   37,   39,   40,   41,   39,   39,
+       24,   28,   29,   24,   30,   31,   24,   24,   24,   32,
+       24,   33,   24,   34,   36,   37,   36,   37,   74,   88,
+       75,   82,   38,   83,   38,   39,   40,   41,   39,   39,
        39,   39,   39,   39,   39,   42,   39,   39,   43,   43,
        39,   39,   43,   43,   43,   43,   43,   43,   43,   43,
        43,   43,   43,   43,   43,   43,   43,   43,   39,   47,
-       52,   53,   65,   47,   88,   75,   55,   72,   67,   61,
-       48,   55,   68,   51,   48,   61,   51,   56,   73,   52,
+       52,   53,   66,   47,   88,   75,   55,   72,   68,   62,
+       48,   55,   69,   51,   48,   62,   51,   56,   73,   52,
 
        53,   73,   87,   73,   84,   55,   73,   83,   83,   86,
        55,   46,   85,   46,   46,   49,   84,   49,   49,   54,
-       54,   71,   81,   68,   80,   78,   79,   78,   77,   76,
-       88,   46,   44,   71,   70,   69,   66,   64,   63,   62,
+       54,   71,   69,   81,   79,   80,   79,   78,   77,   76,
+       88,   46,   44,   71,   70,   67,   65,   64,   63,   62,
        61,   60,   59,   58,   88,   57,   45,   45,   51,   50,
-       45,   44,   88,   38,   38,   34,   34,   11,   88,   88,
+       45,   44,   88,   35,   35,   11,   88,   88,   88,   88,
        88,   88,   88,   88,   88,   88,   88,   88,   88,   88,
        88,   88,   88,   88,   88,   88,   88,   88,   88,   88,
-       88,   88,   88,   88,   88,   88,   88,   88,   88,   88,
-       88,   88
-
+       88,   88,   88,   88,   88,   88,   88,   88,   88,   88
     } ;
 
-static yyconst flex_int16_t yy_chk[193] =
+static yyconst flex_int16_t yy_chk[191] =
     {   0,
         1,    1,    1,    1,    1,    1,    1,    1,    1,    1,
         1,    1,    1,    1,    1,    1,    1,    1,    1,    1,
@@ -475,20 +473,18 @@ static yyconst flex_int16_t yy_chk[193] =
         9,    9,    9,    9,    9,    9,    9,    9,    9,    9,
         9,    9,    9,    9,    9,    9,    9,    9,    9,    9,
         9,    9,    9,    9,    9,    9,    9,    9,    9,   16,
-       20,   20,   34,   46,   75,   75,   20,   93,   36,   34,
-       16,   20,   36,   51,   46,   36,   52,   92,   51,   53,
+       20,   20,   35,   46,   75,   75,   20,   93,   37,   35,
+       16,   20,   37,   51,   46,   37,   52,   92,   51,   53,
 
-       53,   52,   85,   51,   84,   53,   52,   83,   82,   81,
+       53,   52,   85,   51,   84,   53,   52,   83,   82,   78,
        53,   89,   77,   89,   89,   90,   74,   90,   90,   91,
-       91,   71,   70,   69,   67,   66,   63,   60,   59,   58,
-       54,   48,   44,   40,   38,   37,   35,   33,   32,   31,
+       91,   71,   70,   68,   67,   64,   61,   60,   59,   58,
+       54,   48,   44,   40,   38,   36,   34,   33,   32,   31,
        30,   29,   28,   27,   26,   25,   23,   22,   19,   18,
-       15,   13,   11,    8,    7,    4,    3,   88,   88,   88,
+       15,   13,   11,    4,    3,   88,   88,   88,   88,   88,
        88,   88,   88,   88,   88,   88,   88,   88,   88,   88,
        88,   88,   88,   88,   88,   88,   88,   88,   88,   88,
-       88,   88,   88,   88,   88,   88,   88,   88,   88,   88,
-       88,   88
-
+       88,   88,   88,   88,   88,   88,   88,   88,   88,   88
     } ;
 
 /* The intent behind this definition is that it'll catch
@@ -563,7 +559,7 @@ static yyconst flex_int16_t yy_chk[193] =
 
 
 
-#line 567 "scanner.cpp"
+#line 563 "scanner.cpp"
 
 #define INITIAL 0
 #define matchof 1
@@ -822,7 +818,7 @@ YY_DECL
     }
 
 
-#line 826 "scanner.cpp"
+#line 822 "scanner.cpp"
 
        if ( !yyg->yy_init )
                {
@@ -958,7 +954,7 @@ case YY_STATE_EOF(help):
 case 7:
 YY_RULE_SETUP
 #line 140 "scanner.l"
-{ BEGIN(help); return HELP; }
+{ if (YYSTATE == cmdstart) { BEGIN(help); } return HELP; }
        YY_BREAK
 
 case 8:
@@ -1060,7 +1056,7 @@ YY_RULE_SETUP
 #line 166 "scanner.l"
 YY_FATAL_ERROR( "flex scanner jammed" );
        YY_BREAK
-#line 1064 "scanner.cpp"
+#line 1060 "scanner.cpp"
 
        case YY_END_OF_BUFFER:
                {

diff --git a/src/gromacs/selection/scanner.l b/src/gromacs/selection/scanner.l
index 82035d809974534640c4b9aa19029dba69466d3d..1039f1c7b8f694f4bd9508a84f21fa1e9d11bbef 100644 (file)
--- a/src/gromacs/selection/scanner.l
+++ b/src/gromacs/selection/scanner.l
@@ -137,7 +137,7 @@ COMMENT    (#.*)
 <cmdstart><<EOF>> { state->bCmdStart = true; yyterminate(); }
 <<EOF>>         { state->bCmdStart = true; return CMD_SEP; }
 
-<cmdstart>help  { BEGIN(help); return HELP; }
+help            { if (YYSTATE == cmdstart) { BEGIN(help); } return HELP; }
 <help>{
 [[:blank:]]+
 {IDENTIFIER}    { yylval->str = gmx_strndup(yytext, yyleng); return HELP_TOPIC; }

diff --git a/src/gromacs/selection/scanner_internal.cpp b/src/gromacs/selection/scanner_internal.cpp
index b7a6691b50acc9c57b44a31a59fce883396c6c50..de589f9e530390aa81cbae07a68a93fb8dd3f37f 100644 (file)
--- a/src/gromacs/selection/scanner_internal.cpp
+++ b/src/gromacs/selection/scanner_internal.cpp
@@ -64,6 +64,7 @@
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/gmxassert.h"
 #include "gromacs/utility/messagestringcollector.h"
+#include "gromacs/utility/stringutil.h"
 
 #include "parsetree.h"
 #include "selectioncollection-impl.h"
@@ -287,9 +288,9 @@ _gmx_sel_lexer_process_identifier(YYSTYPE *yylval, char *yytext, size_t yyleng,
     /* Reserved symbols should have been caught earlier */
     if (symtype == gmx::SelectionParserSymbol::ReservedSymbol)
     {
-        GMX_ERROR_NORET(gmx::eeInternalError,
-                        "Mismatch between tokenizer and reserved symbol table");
-        return INVALID;
+        GMX_THROW(gmx::InternalError(gmx::formatString(
+                        "Mismatch between tokenizer and reserved symbol table (for '%s')",
+                        symbol->name().c_str())));
     }
     /* For variable symbols, return the type of the variable value */
     if (symtype == gmx::SelectionParserSymbol::VariableSymbol)

diff --git a/src/gromacs/selection/tests/selectioncollection.cpp b/src/gromacs/selection/tests/selectioncollection.cpp
index 7cf179f7884403b57c577b56ef5008d3a34795a3..660af1e6335572d048bb3ce58a41d95f20174dc1 100644 (file)
--- a/src/gromacs/selection/tests/selectioncollection.cpp
+++ b/src/gromacs/selection/tests/selectioncollection.cpp
@@ -380,6 +380,12 @@ TEST_F(SelectionCollectionTest, HandlesMissingMethodParamValue3)
                  gmx::InvalidInputError);
 }
 
+TEST_F(SelectionCollectionTest, HandlesHelpKeywordInInvalidContext)
+{
+    EXPECT_THROW(sc_.parseFromString("resname help"),
+                 gmx::InvalidInputError);
+}
+
 // TODO: Tests for more parser errors
 
 TEST_F(SelectionCollectionTest, RecoversFromUnknownGroupReference)