ENH: code simplification, improvements for reading dictionary variables

- Now accept '/' when reading variables without requiring
  a surrounding '{}'

- fix some degenerate parsing cases when the first character is
  already bad.

  Eg, $"abc" would have previously parsed as a <$"> variable, even
  although a double quote is not a valid variable character.

  Now emits a warning and parses as a '$' token and a string token.

applications/test/dictionary/testSubkeyword

@@ -26,6 +26,11 @@ subdict
     key2 b;
 }
 
+// This parses as a variable without a name (== plain word)
+// followed by a string
+
+keyXYZ   $"test";
+
 update
 {
     key1 val1b;
@@ -66,6 +71,9 @@ key3slash ${/subdict/key1};
 key3 ${^update.subdict.key3};
 key4 ${:update.subdict...subdict.key1};
 
+key3_a   ${/subdict/key1};
+key3_b   $/subdict/key1;
+
 // This will not work, but globs would be interesting:
 #remove "/update/subdict/key*"
 

src/OpenFOAM/db/IOstreams/Sstreams/ISstream.C

@@ -46,6 +46,13 @@ inline static Foam::word charToWord(char c)
     return Foam::word(std::string(1, c), false);
 }
 
+
+// Permit slash-scoping of entries
+static inline bool validVariableChar(char c)
+{
+    return (Foam::word::valid(c) || c == '/');
+}
+
 } // End anonymous namespace
 
 
@@ -255,7 +262,7 @@ Foam::Istream& Foam::ISstream::read(token& t)
         }
 
         // Dictionary variable (as rvalue)
-        case '$':
+        case token::DOLLAR :
         {
             char nextC;
             if (read(nextC).bad())
@@ -263,9 +270,9 @@ Foam::Istream& Foam::ISstream::read(token& t)
                 // Return lone '$' as word
                 t = charToWord(c);
             }
-            else if (nextC == token::BEGIN_BLOCK)
+            else
             {
-                // Put back so that "${" is included in the variable
+                // Put back both so that '$...' is included in the variable
                 putback(nextC);
                 putback(c);
 
@@ -280,14 +287,6 @@ Foam::Istream& Foam::ISstream::read(token& t)
                     t.setType(token::tokenType::VARIABLE);
                 }
             }
-            else
-            {
-                // Word/variable beginning with '$', but without "{}"
-
-                putback(nextC);
-                putback(c);
-                readWordToken(t);
-            }
 
             return *this;
         }
@@ -331,7 +330,7 @@ Foam::Istream& Foam::ISstream::read(token& t)
                 buf[nChar++] = c;
                 if (nChar == maxLen)
                 {
-                    // runaway argument - avoid buffer overflow
+                    // Runaway argument - avoid buffer overflow
                     buf[maxLen-1] = '\0';
 
                     FatalIOErrorInFunction(*this)
@@ -343,7 +342,7 @@ Foam::Istream& Foam::ISstream::read(token& t)
                     return *this;
                 }
             }
-            buf[nChar] = '\0';
+            buf[nChar] = '\0';  // Terminate string
 
             setState(is_.rdstate());
             if (is_.bad())
@@ -408,10 +407,15 @@ Foam::Istream& Foam::ISstream::read(word& str)
     static char buf[maxLen];
 
     unsigned nChar = 0;
-    unsigned depth = 0;  // Track depth of "()" nesting
+    unsigned depth = 0;  // Track depth of (..) nesting
     char c;
 
-    while (get(c) && word::valid(c))
+    while
+    (
+        (nChar < maxLen)
+     && get(c)
+     && word::valid(c)
+    )
     {
         if (c == token::BEGIN_LIST)
         {
@@ -419,19 +423,17 @@ Foam::Istream& Foam::ISstream::read(word& str)
         }
         else if (c == token::END_LIST)
         {
-            if (depth)
+            if (!depth)
             {
+                break;  // Closed ')' without a '(' ? ... stop
+            }
             --depth;
         }
-            else
-            {
-                // Had ')' without a previous '(' ... stop
-                break;
-            }
-        }
 
         buf[nChar++] = c;
-        if (nChar == maxLen)
+    }
+
+    if (nChar >= maxLen)
     {
         buf[errLen] = '\0';
 
@@ -442,18 +444,16 @@ Foam::Istream& Foam::ISstream::read(word& str)
 
         return *this;
     }
-    }
 
-    // Terminate string with nul char
+    buf[nChar] = '\0';  // Terminate string
-    buf[nChar] = '\0';
 
-    // We could probably skip this check
     if (bad())
     {
+        // Could probably skip this check
         buf[errLen] = '\0';
 
         FatalIOErrorInFunction(*this)
-            << "problem while reading word '" << buf << "...' after "
+            << "Problem while reading word '" << buf << "...' after "
             << nChar << " characters\n"
             << exit(FatalIOError);
 
@@ -463,13 +463,14 @@ Foam::Istream& Foam::ISstream::read(word& str)
     if (nChar == 0)
     {
         FatalIOErrorInFunction(*this)
-            << "invalid first character found : " << c
+            << "Invalid first character found : " << c
             << exit(FatalIOError);
     }
     else if (depth)
     {
         IOWarningInFunction(*this)
-            << "Missing " << depth << " closing ')' while parsing" << nl << nl
+            << "Missing " << depth
+            << " closing ')' while parsing" << nl << nl
             << buf << nl << endl;
     }
 
@@ -510,7 +511,11 @@ Foam::Istream& Foam::ISstream::read(string& str)
     unsigned nChar = 0;
     bool escaped = false;
 
-    while (get(c))
+    while
+    (
+        (nChar < maxLen)
+     && get(c)
+    )
     {
         if (c == token::END_STRING)
         {
@@ -522,8 +527,7 @@ Foam::Istream& Foam::ISstream::read(string& str)
             else
             {
                 // Done reading
-                buf[nChar] = '\0';
+                str.assign(buf, nChar);
-                str = buf;
                 return *this;
             }
         }
@@ -556,7 +560,9 @@ Foam::Istream& Foam::ISstream::read(string& str)
         }
 
         buf[nChar++] = c;
-        if (nChar == maxLen)
+    }
+
+    if (nChar >= maxLen)
     {
         buf[errLen] = '\0';
 
@@ -567,14 +573,12 @@ Foam::Istream& Foam::ISstream::read(string& str)
 
         return *this;
     }
-    }
-
 
     // Don't worry about a dangling backslash if string terminated prematurely
     buf[errLen] = buf[nChar] = '\0';
 
     FatalIOErrorInFunction(*this)
-        << "problem while reading string \"" << buf << "...\""
+        << "Problem while reading string \"" << buf << "...\""
         << exit(FatalIOError);
 
     return *this;
@@ -587,32 +591,50 @@ Foam::Istream& Foam::ISstream::readVariable(std::string& str)
     static char buf[maxLen];
 
     unsigned nChar = 0;
-    unsigned depth = 0;  // Track depth of "{}" nesting
+    unsigned depth = 0; // Track depth of (..) or {..} nesting
     char c;
 
-    if (!get(c) || c != '$')
+    // First character must be '$'
+    if (!get(c) || c != token::DOLLAR)
     {
         FatalIOErrorInFunction(*this)
-            << "invalid first character found : " << c
+            << "Invalid first character found : " << c << nl
             << exit(FatalIOError);
     }
-
     buf[nChar++] = c;
 
-    // Read next character to see if '{'
+    // Next character should also exist.
-    if (get(c) && c == token::BEGIN_BLOCK)
+    // This should never fail, since it was checked before calling.
+    if (!get(c))
     {
-        buf[nChar++] = c;
+        str.assign(buf, nChar);
-        ++depth;  // Starts with '{'
+
+        IOWarningInFunction(*this)
+            << "Truncated variable name : " << str << nl;
+
+        return *this;
+    }
+    buf[nChar++] = c;
+
+    char endChar = token::END_LIST;
+
+    if (c == token::BEGIN_BLOCK)
+    {
+        // Processing ${...} style
+
+        endChar = token::END_BLOCK;
+        ++depth;
+
+        // Could check that the next char is good and not one of '{}'
+        // since this would indicate "${}", "${{..." or truncated "${"
 
-        // Also allow '/' between ${...} blocks for slash-scoping of entries
         while
         (
-            get(c)
+            (nChar < maxLen) && get(c)
-         && (
+         &&
-                c == token::BEGIN_BLOCK
+            (
-             || c == token::END_BLOCK
+                validVariableChar(c)
-             || word::valid(c) || c == '/'
+             || (c == token::BEGIN_BLOCK || c == token::END_BLOCK)
             )
         )
         {
@@ -622,19 +644,54 @@ Foam::Istream& Foam::ISstream::readVariable(std::string& str)
             }
             else if (c == token::END_BLOCK)
             {
-                if (depth)
+                if (!depth)
                 {
+                    break;  // Closed '}' without a '{' ? ... stop
+                }
                 --depth;
             }
-                else
+
-                {
+            buf[nChar++] = c;
-                    // Had '}' without a previous '{' ... stop
-                    break;
         }
     }
+    else if (validVariableChar(c))
+    {
+        // Processing $var style
+
+        while
+        (
+            (nChar < maxLen) && get(c)
+         && (validVariableChar(c))
+        )
+        {
+            if (c == token::BEGIN_LIST)
+            {
+                ++depth;
+            }
+            else if (c == token::END_LIST)
+            {
+                if (!depth)
+                {
+                    break;  // Closed ')' without a '(' ? ... stop
+                }
+                --depth;
+            }
 
             buf[nChar++] = c;
-            if (nChar == maxLen)
+        }
+    }
+    else
+    {
+        // Invalid character. Terminate string (for message) without
+        // including the invalid character in the count.
+
+        buf[nChar--] = '\0';
+
+        IOWarningInFunction(*this)
+            << "Bad variable name: " << buf << nl << endl;
+    }
+
+    if (nChar >= maxLen)
     {
         buf[errLen] = '\0';
 
@@ -645,55 +702,27 @@ Foam::Istream& Foam::ISstream::readVariable(std::string& str)
 
         return *this;
     }
-        }
-    }
-    else
-    {
-        buf[nChar++] = c;
 
-        while (get(c) && word::valid(c))
+    buf[nChar] = '\0';  // Terminate string
-        {
-            buf[nChar++] = c;
-            if (nChar == maxLen)
-            {
-                buf[errLen] = '\0';
 
-                FatalIOErrorInFunction(*this)
-                    << "variable '" << buf << "...'\n"
-                    << "    is too long (max. " << maxLen << " characters)"
-                    << exit(FatalIOError);
-
-                return *this;
-            }
-        }
-    }
-
-    // Terminate string with nul char
-    buf[nChar] = '\0';
-
-    // we could probably skip this check
     if (bad())
     {
+        // Could probably skip this check
         buf[errLen] = '\0';
 
         FatalIOErrorInFunction(*this)
-            << "problem while reading string '" << buf << "...' after "
+            << "Problem while reading variable '" << buf << "...' after "
             << nChar << " characters\n"
             << exit(FatalIOError);
 
         return *this;
     }
 
-    if (nChar == 0)
+    if (depth)
-    {
-        FatalIOErrorInFunction(*this)
-            << "invalid first character found : " << c
-            << exit(FatalIOError);
-    }
-    else if (depth)
     {
         IOWarningInFunction(*this)
-            << "Missing " << depth << " closing '}' while parsing" << nl << nl
+            << "Missing " << depth
+            << " closing '" << endChar << "' while parsing" << nl << nl
             << buf << nl << endl;
     }
 
@@ -740,12 +769,11 @@ Foam::Istream& Foam::ISstream::readVerbatim(std::string& str)
         }
     }
 
-
+    // Truncated terminated prematurely
-    // Don't worry about a dangling backslash if string terminated prematurely
     buf[errLen] = buf[nChar] = '\0';
 
     FatalIOErrorInFunction(*this)
-        << "problem while reading string \"" << buf << "...\""
+        << "Problem while reading string \"" << buf << "...\""
         << exit(FatalIOError);
 
     return *this;

src/OpenFOAM/db/dictionary/entry/entryIO.C

@@ -192,7 +192,7 @@ bool Foam::entry::New
     }
 
 
-    if (keyword[0] == '#')
+    if (keyword[0] == token::HASH)
     {
         // Function entry - #function
 
@@ -215,7 +215,7 @@ bool Foam::entry::New
     }
 
 
-    if (!disableFunctionEntries && keyword[0] == '$')
+    if (!disableFunctionEntries && keyword[0] == token::DOLLAR)
     {
         // Substitution entry - $variable
 

src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntry.C

@@ -2,7 +2,7 @@
   =========                 |
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
-    \\  /    A nd           | Copyright (C) 2017 OpenCFD Ltd.
+    \\  /    A nd           | Copyright (C) 2017-2019 OpenCFD Ltd.
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
                             | Copyright (C) 2011-2016 OpenFOAM Foundation
@@ -85,14 +85,15 @@ bool Foam::primitiveEntry::expandVariable
     if (!eptr)
     {
         // Not found - revert to environment variable
-        const string str(getEnv(varName));
+        const string str(Foam::getEnv(varName));
 
         if (str.empty())
         {
             FatalIOErrorInFunction(dict)
                 << "Illegal dictionary entry or environment variable name "
-                << varName << endl << "Valid dictionary entries are "
+                << varName << nl
-                << dict.toc() << exit(FatalIOError);
+                << "Known dictionary entries: " << dict.toc() << nl
+                << exit(FatalIOError);
 
             return false;
         }

src/OpenFOAM/db/dictionary/primitiveEntry/primitiveEntryIO.C

@@ -2,7 +2,7 @@
   =========                 |
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
-    \\  /    A nd           | Copyright (C) 2017-2018 OpenCFD Ltd.
+    \\  /    A nd           | Copyright (C) 2017-2019 OpenCFD Ltd.
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
                             | Copyright (C) 2011-2015 OpenFOAM Foundation
@@ -81,12 +81,8 @@ bool Foam::primitiveEntry::acceptToken
         accept =
         (
             disableFunctionEntries
-         || key.size() <= 3
+         || key.size() == 1
-         || !(
+         || !(key[0] == '$' && expandVariable(key.substr(1), dict))
-                key[0] == '$'
-             && key[1] == token::BEGIN_BLOCK
-             && expandVariable(key.substr(1), dict)
-            )
         );
     }
 

src/OpenFOAM/primitives/strings/stringOps/stringOps.C

@@ -277,7 +277,9 @@ namespace
 //
 // Similar to word::valid(), except we don't have the benefit of a parser
 // to filter out other unacceptable entries for us.
-
+//
+// Does not currently accept '/' in a variable name.
+// We would like "$file/$name" to expand as two variables.
 static inline bool validVariableChar(char c)
 {
     return

tutorials/verificationAndValidation/turbulentInflow/system/sampling

@@ -20,7 +20,7 @@ fieldAverage1
     type                fieldAverage;
     libs                (fieldFunctionObjects);
     writeControl        writeTime;
-    timeStart           ${/timeStart};
+    timeStart           $/timeStart;
 
     fields
     (
@@ -39,7 +39,7 @@ inletSampling
     type                sets;
     libs                (sampling);
     writeControl        writeTime;
-    timeStart           ${/timeStart};
+    timeStart           $/timeStart;
 
     interpolationScheme cellPoint;
     setFormat           raw;