[parser] Check for properly balanced #end, braces, parentheses etc.

Author: c-lipka

changes.txt

@@ -105,6 +105,9 @@ New Features
     and the bit depth.
   - Support for blue noise dithering has been added, plus a couple more error
     diffusion dithering filters.
+  - The parser now checks for proper balancing of `#end` directives, braces,
+    parentheses etc. within each include file, and will report any imbalance
+    via warnings or, in case of `#end`, outright errors.
 
 Performance Improvements
 ------------------------

source/parser/parser.cpp

@@ -769,7 +769,15 @@ void Parser::Parse_End(TokenId openTokenId, TokenId expectTokenId)
     {
         POV_PARSER_ASSERT(!maBraceStack.empty());
         POV_PARSER_ASSERT(openTokenId == maBraceStack.back().openToken);
+
+        if (!maIncludeStack.empty() && (maBraceStack.size() <= maIncludeStack.back().braceStackSize))
+        {
+            BraceStackEntry& braceStackEntry = maBraceStack.back();
+            // Include file has closed more braces/parentheses/etc. than it has opened.
+            Warning("Unbalanced %s in include file", Get_Token_String(CurrentTokenId()));
+        }
         maBraceStack.pop_back();
+
         return;
     }
 
@@ -10611,6 +10619,18 @@ void Parser::Warning(const char *format,...)
     Warning(kWarningGeneral, localvsbuffer);
 }
 
+void Parser::Warning(const MessageContext& loc, const char *format, ...)
+{
+    va_list marker;
+    char localvsbuffer[1024];
+
+    va_start(marker, format);
+    std::vsnprintf(localvsbuffer, sizeof(localvsbuffer), format, marker);
+    va_end(marker);
+
+    Warning(kWarningGeneral, loc, localvsbuffer);
+}
+
 void Parser::Warning(WarningLevel level, const char *format,...)
 {
     POV_PARSER_ASSERT(level >= kWarningGeneral);
@@ -10628,6 +10648,20 @@ void Parser::Warning(WarningLevel level, const char *format,...)
         mMessageFactory.Warning(level, "%s", localvsbuffer);
 }
 
+void Parser::Warning(WarningLevel level, const MessageContext& loc, const char *format, ...)
+{
+    POV_PARSER_ASSERT(level >= kWarningGeneral);
+
+    va_list marker;
+    char localvsbuffer[1024];
+
+    va_start(marker, format);
+    std::vsnprintf(localvsbuffer, sizeof(localvsbuffer), format, marker);
+    va_end(marker);
+
+    mMessageFactory.WarningAt(level, loc, "%s", localvsbuffer);
+}
+
 void Parser::VersionWarning(unsigned int sinceVersion, const char *format,...)
 {
     if(sceneData->EffectiveLanguageVersion() >= sinceVersion)

source/parser/parser.h

@@ -421,7 +421,9 @@ class Parser
         void SendFatalError(Exception& e);
 
         void Warning(const char *format,...);
+        void Warning(const MessageContext& loc, const char *format, ...);
         void Warning(WarningLevel level, const char *format,...);
+        void Warning(WarningLevel level, const MessageContext& loc, const char *format, ...);
         void VersionWarning(unsigned int sinceVersion, const char *format,...);
         void PossibleError(const char *format,...);
         void Error(const char *format,...);
@@ -660,7 +662,17 @@ class Parser
         POV_LONG    mTokenCount;
         int         mTokensSinceLastProgressReport;
 
-        vector<RawTokenizer::HotBookmark> maIncludeStack;
+        struct IncludeStackEntry
+        {
+            RawTokenizer::HotBookmark   returnToBookmark;
+            int                         condStackSize;
+            int                         braceStackSize;
+
+            IncludeStackEntry(const RawTokenizer::HotBookmark& rtb, int css, int bss) :
+                returnToBookmark(rtb), condStackSize(css), braceStackSize(bss)
+            {}
+        };
+        vector<IncludeStackEntry> maIncludeStack;
 
         struct CS_ENTRY
         {

source/parser/parser_tokenizer.cpp

@@ -294,8 +294,12 @@ void Parser::Get_Token ()
 
         if (!GetRawToken(mToken.raw, fastForwardToDirective))
         {
+            // End of current token stream reached.
+
             if (maIncludeStack.empty())
             {
+                // Not in an include file, i.e. end of main scene file reached.
+
                 if (Cond_Stack.size() != 1)
                     Error("End of file reached but #end expected.");
 
@@ -305,12 +309,24 @@ void Parser::Get_Token ()
             }
 
             // Returning from an include file.
+            // NB: End of macro is marked by `#end` rather than EOF, so it won't take us here.
 
             Got_EOF=false;
 
             mSymbolStack.PopTable();
 
-            GoToBookmark(maIncludeStack.back()); // TODO handle errors
+            if (Cond_Stack.size() != maIncludeStack.back().condStackSize)
+                Error("Unbalanced #end directives in include file.");
+            if (maBraceStack.size() > maIncludeStack.back().braceStackSize)
+            {
+                // Include file has opened more braces/parentheses/etc. than it has closed.
+                for (size_t i = maIncludeStack.back().braceStackSize; i < maBraceStack.size(); ++i)
+                {
+                    BraceStackEntry& braceStackEntry = maBraceStack[i];
+                    Warning(braceStackEntry, "Unbalanced %s in include file", Get_Token_String(braceStackEntry.openToken));
+                }
+            }
+            GoToBookmark(maIncludeStack.back().returnToBookmark); // TODO handle errors
             maIncludeStack.pop_back();
 
             continue;
@@ -1881,9 +1897,9 @@ void Parser::Parse_Version()
 
         if (maIncludeStack.empty())
             Error("As of POV-Ray v3.7, the '#version' directive must be the first non-comment "
-                    "statement in the scene file. To indicate that your scene will dynamically "
-                    "adapt to whatever POV-Ray version is actually used, start your scene with "
-                    "'#version version;'.");
+                  "statement in the scene file. To indicate that your scene will dynamically "
+                  "adapt to whatever POV-Ray version is actually used, start your scene with "
+                  "'#version version;'.");
     }
 
     // Initialize various defaults depending on language version specified.
@@ -1896,7 +1912,7 @@ void Parser::Parse_Version()
 
     if (sceneData->explicitNoiseGenerator == false)
         sceneData->noiseGenerator = (sceneData->EffectiveLanguageVersion() < 350 ?
-                                        kNoiseGen_Original : kNoiseGen_RangeCorrected);
+                                     kNoiseGen_Original : kNoiseGen_RangeCorrected);
     // [CLi] if assumed_gamma is not specified in a pre-v3.7 scene, gammaMode defaults to kPOVList_GammaMode_None;
     // this is enforced later anyway after parsing, but we may need this information /now/ during parsing already
     switch (sceneData->gammaMode)
@@ -1979,7 +1995,7 @@ void Parser::Open_Include()
 
 void Parser::Skip_Tokens(COND_TYPE cond)
 {
-    int Temp       = Cond_Stack.size();
+    auto Temp      = Cond_Stack.size();
     bool Prev_Skip = Skipping;
 
     Skipping = true;
@@ -3267,7 +3283,7 @@ void Parser::IncludeHeader(const UCS2String& formalFileName)
     if (formalFileName.empty())
         return;
 
-    maIncludeStack.push_back(mTokenizer.GetHotBookmark());
+    maIncludeStack.emplace_back(mTokenizer.GetHotBookmark(), int(Cond_Stack.size()), int(maBraceStack.size()));
 
     shared_ptr<IStream> is = Locate_File (formalFileName.c_str(),POV_File_Text_INC,actualFileName,true);
     if (is == nullptr)