From 19906b6501a1d5b35d2bd26817ca724c9528b8d2 Mon Sep 17 00:00:00 2001
From: Tim Foley <tfoley@nvidia.com>
Date: Mon, 19 Jun 2017 14:21:14 -0700
Subject: Fixes for preprocessor conditionals that use macros

The basic underlying problem here is that the preprocessor tries to linger at the end of an input stream until it is sure it is time to advance.
An input stream can include raw input files, or the expansion of a macro or macro argument.
This was originally done to deal with not getting good end-of-line tokens when in directives (that issue has been fixed), but it is now a legacy issue that should probably be removed (but I am wary of making such a sweeping change).

The problem that arises is that some code depends on what the actual input stream is (e.g., when turning conditionals on/off), and so we need to be careful.
The bugs that this change affects arise when a `#if` or `#elseif` conditional expression *ends* with a macro expansion:

    #define FOO 2
    #if 2 == FOO
      ...
    #endif

When we try to start the preprocessor conditional block the "active" stream is still the expansion of `FOO`, when we needed it to be the input file.

We fix this for now by snapshotting the input stream at the start of the directive, but a better long term fix would be to fix up this weird end-of-input behavior.
---
 source/slang/preprocessor.cpp | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/source/slang/preprocessor.cpp b/source/slang/preprocessor.cpp
index 6e0906d9e..00abb3fe5 100644
--- a/source/slang/preprocessor.cpp
+++ b/source/slang/preprocessor.cpp
@@ -1008,10 +1008,12 @@ static void DestroyConditional(PreprocessorConditional* conditional)
 }
 
 // Start a preprocessor conditional, with an initial enable/disable state.
-static void BeginConditional(PreprocessorDirectiveContext* context, bool enable)
+static void beginConditional(
+    PreprocessorDirectiveContext*   context,
+    PreprocessorInputStream*        inputStream,
+    bool                            enable)
 {
     Preprocessor* preprocessor = context->preprocessor;
-    PreprocessorInputStream* inputStream = preprocessor->inputStream;
     assert(inputStream);
 
     PreprocessorConditional* conditional = CreateConditional(preprocessor);
@@ -1043,6 +1045,14 @@ static void BeginConditional(PreprocessorDirectiveContext* context, bool enable)
     inputStream->conditional = conditional;
 }
 
+// Start a preprocessor conditional, with an initial enable/disable state.
+static void beginConditional(
+    PreprocessorDirectiveContext*   context,
+    bool                            enable)
+{
+    beginConditional(context, context->preprocessor->inputStream, enable);
+}
+
 //
 // Preprocessor Conditional Expressions
 //
@@ -1302,11 +1312,16 @@ static PreprocessorExpressionValue ParseAndEvaluateExpression(PreprocessorDirect
 // Handle a `#if` directive
 static void HandleIfDirective(PreprocessorDirectiveContext* context)
 {
+    // Record current inpu stream in case preprocessor expression
+    // changes the input stream to a macro expansion while we
+    // are parsing.
+    auto inputStream = context->preprocessor->inputStream;
+
     // Parse a preprocessor expression.
     PreprocessorExpressionValue value = ParseAndEvaluateExpression(context);
 
     // Begin a preprocessor block, enabled based on the expression.
-    BeginConditional(context, value != 0);
+    beginConditional(context, inputStream, value != 0);
 }
 
 // Handle a `#ifdef` directive
@@ -1319,7 +1334,7 @@ static void HandleIfDefDirective(PreprocessorDirectiveContext* context)
     String name = nameToken.Content;
 
     // Check if the name is defined.
-    BeginConditional(context, LookupMacro(context, name) != NULL);
+    beginConditional(context, LookupMacro(context, name) != NULL);
 }
 
 // Handle a `#ifndef` directive
@@ -1332,7 +1347,7 @@ static void HandleIfNDefDirective(PreprocessorDirectiveContext* context)
     String name = nameToken.Content;
 
     // Check if the name is defined.
-    BeginConditional(context, LookupMacro(context, name) == NULL);
+    beginConditional(context, LookupMacro(context, name) == NULL);
 }
 
 // Handle a `#else` directive
@@ -1376,6 +1391,11 @@ static void HandleElseDirective(PreprocessorDirectiveContext* context)
 // Handle a `#elif` directive
 static void HandleElifDirective(PreprocessorDirectiveContext* context)
 {
+    // Need to grab current input stream *before* we try to parse
+    // the conditional expression.
+    PreprocessorInputStream* inputStream = context->preprocessor->inputStream;
+    assert(inputStream);
+
     // HACK(tfoley): handle an empty `elif` like an `else` directive
     //
     // This is the behavior expected by at least one input program.
@@ -1390,9 +1410,6 @@ static void HandleElifDirective(PreprocessorDirectiveContext* context)
 
     PreprocessorExpressionValue value = ParseAndEvaluateExpression(context);
 
-    PreprocessorInputStream* inputStream = context->preprocessor->inputStream;
-    assert(inputStream);
-
     // if we aren't inside a conditional, then error
     PreprocessorConditional* conditional = inputStream->conditional;
     if (!conditional)
-- 
cgit v1.2.3