New conditional C Scanner; Improvements for virtual C Pre-Processor

5 files changed, 186 insertions, 57 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index c02bb36..c13e3de 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -33,6 +33,17 @@ RELEASE 3.1.0.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE
     - Updated manpage scons.xml to fix a nested list problem
     - Updated doc terminionly: use prepend instead of append as appropriate
 
+  From Ivan Kravets, PlatformIO
+    - New conditional C Scanner (`SCons.Scanner.C.CConditionalScanner()`)
+      which interprets C/C Preprocessor conditional syntax (#ifdef, #if, #else,
+      #elif, #define, etc.)
+    - Improvements for virtual C Pre-Processor:
+      * Handle UNSIGNED LONG and LONG numeric constants in DEC (keep support for HEX)
+      * Skip unrecognized directives, such as `#if( defined ...)`
+      * Ignore `#include DYNAMIC_INCLUDE` directive that depends on a dynamic
+        macro which is not located in a state TABLE.
+      * Cleanup CPP expressions before evaluating (strip comments, carriage returns)
+
 RELEASE 3.0.1 - Mon, 12 Nov 2017 15:31:33 -0700
 
   From Daniel Moody:
diff --git a/src/engine/SCons/Scanner/C.py b/src/engine/SCons/Scanner/C.py
index c2e50a6..db7463f 100644
--- a/src/engine/SCons/Scanner/C.py
+++ b/src/engine/SCons/Scanner/C.py
@@ -1,6 +1,6 @@
 """SCons.Scanner.C
 
-This module implements the dependency scanner for C/C++ code. 
+This module implements the dependency scanner for C/C++ code.
 
 """
 
@@ -72,7 +72,11 @@ def dictify_CPPDEFINES(env):
         result = {}
         for c in cppdefines:
             if SCons.Util.is_Sequence(c):
-                result[c[0]] = c[1]
+                # handle tuple with 1 item (e.g. tuple("DEFINE", ))
+                if len(c) > 1:
+                    result[c[0]] = c[1]
+                else:
+                    result[c[0]] = None
             else:
                 result[c] = None
         return result
@@ -108,21 +112,27 @@ class SConsCPPScannerWrapper(object):
     def select(self, node):
         return self
 
+
+def CConditionalScanner():
+    """
+    Return an advanced conditional Scanner instance for scanning source files
+
+    Interprets C/C++ Preprocessor conditional syntax
+    (#ifdef, #if, defined, #else, #elif, etc.).
+    """
+    return SConsCPPScannerWrapper("CScanner", "CPPPATH")
+
+
 def CScanner():
-    """Return a prototype Scanner instance for scanning source files
-    that use the C pre-processor"""
-
-    # Here's how we would (or might) use the CPP scanner code above that
-    # knows how to evaluate #if/#ifdef/#else/#elif lines when searching
-    # for #includes.  This is commented out for now until we add the
-    # right configurability to let users pick between the scanners.
-    #return SConsCPPScannerWrapper("CScanner", "CPPPATH")
-
-    cs = SCons.Scanner.ClassicCPP("CScanner",
-                                  "$CPPSUFFIXES",
-                                  "CPPPATH",
-                                  '^[ \t]*#[ \t]*(?:include|import)[ \t]*(<|")([^>"]+)(>|")')
-    return cs
+    """Return a simplified classic Scanner instance for scanning source files
+
+    Takes into account the type of bracketing used to include the file, and
+    uses classic CPP rules for searching for the files based on the bracketing.
+    """
+    return SCons.Scanner.ClassicCPP(
+        "CScanner", "$CPPSUFFIXES", "CPPPATH",
+        '^[ \t]*#[ \t]*(?:include|import)[ \t]*(<|")([^>"]+)(>|")')
+
 
 # Local Variables:
 # tab-width:4
diff --git a/src/engine/SCons/Scanner/CTests.py b/src/engine/SCons/Scanner/CTests.py
index 9c7df12..94d3dbf 100644
--- a/src/engine/SCons/Scanner/CTests.py
+++ b/src/engine/SCons/Scanner/CTests.py
@@ -27,7 +27,6 @@ import SCons.compat
 
 import collections
 import os
-import sys
 import unittest
 
 import TestCmd
@@ -45,8 +44,11 @@ os.chdir(test.workpath(''))
 # create some source files and headers:
 
 test.write('f1.cpp',"""
-#include \"f1.h\"
+#ifdef INCLUDE_F2 /* multi-line comment */
 #include <f2.h>
+#else
+#include \"f1.h\"
+#endif
 
 int main()
 {
@@ -56,8 +58,15 @@ int main()
 
 test.write('f2.cpp',"""
 #include \"d1/f1.h\"
-#include <d2/f1.h>
+
+#if 5UL < 10 && !defined(DUMMY_MACRO) // some comment
+    #if NESTED_CONDITION
+        #include <d2/f1.h>
+    #endif
+#else
 #include \"f1.h\"
+#endif
+
 #import <f4.h>
 
 int main()
@@ -261,7 +270,7 @@ class CScannerTestCase4(unittest.TestCase):
         deps = s(env.File('f2.cpp'), env, path)
         headers =  ['d1/f1.h', 'f1.h', 'd1/d2/f1.h', 'd1/d2/f4.h']
         deps_match(self, deps, headers)
-        
+
 class CScannerTestCase5(unittest.TestCase):
     def runTest(self):
         """Make sure files in repositories will get scanned"""
@@ -282,7 +291,7 @@ class CScannerTestCase5(unittest.TestCase):
         # Make sure rexists() got called on the file node being
         # scanned, essential for cooperation with VariantDir functionality.
         assert n.GetTag('rexists_called')
-        
+
         headers =  ['f1.h', 'f2.h', 'f3-test.h',
                     'd1/f1.h', 'd1/f2.h', 'd1/f3-test.h']
         deps_match(self, deps, headers)
@@ -340,7 +349,7 @@ class CScannerTestCase9(unittest.TestCase):
 
         # Did we catch the warning associated with not finding fb.h?
         assert to.out
-        
+
         deps_match(self, deps, [ 'fa.h' ])
         test.unlink('fa.h')
 
@@ -440,6 +449,50 @@ class CScannerTestCase15(unittest.TestCase):
             assert suffix in s.get_skeys(env), "%s not in skeys" % suffix
 
 
+class CConditionalScannerTestCase1(unittest.TestCase):
+    def runTest(self):
+        """Find local files with no CPPPATH"""
+        env = DummyEnvironment(CPPPATH=[])
+        s = SCons.Scanner.C.CConditionalScanner()
+        path = s.path(env)
+        deps = s(env.File('f1.cpp'), env, path)
+        headers = ['f1.h']
+        deps_match(self, deps, headers)
+
+
+class CConditionalScannerTestCase2(unittest.TestCase):
+    def runTest(self):
+        """Find local files with no CPPPATH based on #ifdef"""
+        env = DummyEnvironment(CPPPATH=[], CPPDEFINES=["INCLUDE_F2"])
+        s = SCons.Scanner.C.CConditionalScanner()
+        path = s.path(env)
+        deps = s(env.File('f1.cpp'), env, path)
+        headers = ['f2.h', 'fi.h']
+        deps_match(self, deps, headers)
+
+
+class CConditionalScannerTestCase3(unittest.TestCase):
+    def runTest(self):
+        """Find files in explicit subdirectories, ignore missing file"""
+        env = DummyEnvironment(
+            CPPPATH=[test.workpath("d1")],
+            CPPDEFINES=[("NESTED_CONDITION", 1)]
+        )
+        s = SCons.Scanner.C.CConditionalScanner()
+        deps = s(env.File('f2.cpp'), env, s.path(env))
+        headers = ['d1/f1.h', 'd1/d2/f1.h']
+        deps_match(self, deps, headers)
+
+        # disable nested conditions
+        env = DummyEnvironment(
+            CPPPATH=[test.workpath("d1")],
+            CPPDEFINES=[("NESTED_CONDITION", 0)]
+        )
+        s = SCons.Scanner.C.CConditionalScanner()
+        deps = s(env.File('f2.cpp'), env, s.path(env))
+        headers = ['d1/f1.h']
+        deps_match(self, deps, headers)
+
 
 def suite():
     suite = unittest.TestSuite()
@@ -457,6 +510,9 @@ def suite():
     suite.addTest(CScannerTestCase13())
     suite.addTest(CScannerTestCase14())
     suite.addTest(CScannerTestCase15())
+    suite.addTest(CConditionalScannerTestCase1())
+    suite.addTest(CConditionalScannerTestCase2())
+    suite.addTest(CConditionalScannerTestCase3())
     return suite
 
 if __name__ == "__main__":
diff --git a/src/engine/SCons/cpp.py b/src/engine/SCons/cpp.py
index 161f070..42693b9 100644
--- a/src/engine/SCons/cpp.py
+++ b/src/engine/SCons/cpp.py
@@ -99,8 +99,19 @@ e = '^\s*#\s*(' + '|'.join(l) + ')(.*)$'
 # And last but not least, compile the expression.
 CPP_Expression = re.compile(e, re.M)
 
+# A list with RE to cleanup CPP Expressions (tuples)
+# We should remove all comments and carriage returns (\r) before evaluating
+CPP_Expression_Cleaner_List = [
+    "/\*.*\*/",
+    "/\*.*",
+    "//.*",
+    "\r"
+]
+CPP_Expression_Cleaner_RE = re.compile(
+    "\s*(" + "|".join(CPP_Expression_Cleaner_List) + ")")
 
-
+def Cleanup_CPP_Expressions(ts):
+    return [(t[0], CPP_Expression_Cleaner_RE.sub("", t[1])) for t in ts]
 
 #
 # Second "subsystem" of regular expressions that we set up:
@@ -118,7 +129,6 @@ CPP_to_Python_Ops_Dict = {
     '||'        : ' or ',
     '?'         : ' and ',
     ':'         : ' or ',
-    '\r'        : '',
 }
 
 CPP_to_Python_Ops_Sub = lambda m: CPP_to_Python_Ops_Dict[m.group(0)]
@@ -141,12 +151,10 @@ CPP_to_Python_Ops_Expression = re.compile(expr)
 # A separate list of expressions to be evaluated and substituted
 # sequentially, not all at once.
 CPP_to_Python_Eval_List = [
-    ['defined\s+(\w+)',         '"\\1" in __dict__'],
-    ['defined\s*\((\w+)\)',     '"\\1" in __dict__'],
-    ['/\*.*\*/',                ''],
-    ['/\*.*',                   ''],
-    ['//.*',                    ''],
-    ['(0x[0-9A-Fa-f]*)[UL]+',   '\\1'],
+    ['defined\s+(\w+)',                 '"\\1" in __dict__'],
+    ['defined\s*\((\w+)\)',             '"\\1" in __dict__'],
+    ['(0x[0-9A-Fa-f]+)(?:L|UL)?',  '\\1'],
+    ['(\d+)(?:L|UL)?',  '\\1'],
 ]
 
 # Replace the string representations of the regular expressions in the
@@ -284,7 +292,13 @@ class PreProcessor(object):
         global CPP_Expression, Table
         contents = line_continuations.sub('', contents)
         cpp_tuples = CPP_Expression.findall(contents)
-        return  [(m[0],) + Table[m[0]].match(m[1]).groups() for m in cpp_tuples]
+        cpp_tuples = Cleanup_CPP_Expressions(cpp_tuples)
+        result = []
+        for t in cpp_tuples:
+            m = Table[t[0]].match(t[1])
+            if m:
+                result.append((t[0],) + m.groups())
+        return result
 
     def __call__(self, file):
         """
@@ -354,8 +368,10 @@ class PreProcessor(object):
         track #define values.
         """
         t = CPP_to_Python(' '.join(t[1:]))
-        try: return eval(t, self.cpp_namespace)
-        except (NameError, TypeError): return 0
+        try:
+            return eval(t, self.cpp_namespace)
+        except (NameError, TypeError, SyntaxError):
+            return 0
 
     def initialize_result(self, fname):
         self.result = [fname]
@@ -482,7 +498,11 @@ class PreProcessor(object):
         try:
             expansion = int(expansion)
         except (TypeError, ValueError):
-            pass
+            # handle "defined" chain "! (defined (A) || defined (B)" ...
+            if "defined " in expansion:
+                self.cpp_namespace[name] = self.eval_expression(t[2:])
+                return
+
         if args:
             evaluator = FunctionEvaluator(name, args[1:-1], expansion)
             self.cpp_namespace[name] = evaluator
@@ -508,15 +528,21 @@ class PreProcessor(object):
         Default handling of a #include line.
         """
         t = self.resolve_include(t)
+        if not t:
+            return
         include_file = self.find_include_file(t)
-        if include_file:
-            #print("include_file =", include_file)
-            self.result.append(include_file)
-            contents = self.read_file(include_file)
-            new_tuples = [('scons_current_file', include_file)] + \
-                         self.tupleize(contents) + \
-                         [('scons_current_file', self.current_file)]
-            self.tuples[:] = new_tuples + self.tuples
+        if not include_file:
+            return
+        # print("include_file =", include_file)
+        self.result.append(include_file)
+        # handle infinite recursion
+        for t in self.tuples:
+            if t[0] == 'scons_current_file' and t[1] == include_file:
+                return
+        new_tuples = [('scons_current_file', include_file)] + \
+                     self.tupleize(self.read_file(include_file)) + \
+                     [('scons_current_file', self.current_file)]
+        self.tuples[:] = new_tuples + self.tuples
 
     # Date: Tue, 22 Nov 2005 20:26:09 -0500
     # From: Stefan Seefeld <seefeld@sympatico.ca>
@@ -542,18 +568,25 @@ class PreProcessor(object):
 
         This handles recursive expansion of values without "" or <>
         surrounding the name until an initial " or < is found, to handle
-
                 #include FILE
-
-        where FILE is a #define somewhere else."""
-
-        s = t[1]
+        where FILE is a #define somewhere else.
+        """
+        s = t[1].strip()
         while not s[0] in '<"':
-            #print("s =", s)
             try:
                 s = self.cpp_namespace[s]
             except KeyError:
                 m = function_name.search(s)
+
+                # Date: Mon, 28 Nov 2016 17:47:13 UTC
+                # From: Ivan Kravets <ikravets@platformio.org>
+                #
+                # Ignore `#include` directive that depends on dynamic macro
+                # which is not located in state TABLE
+                # For example, `#include MYCONFIG_FILE`
+                if not m:
+                    return None
+
                 s = self.cpp_namespace[m.group(1)]
                 if callable(s):
                     args = function_arg_separator.split(m.group(2))
diff --git a/src/engine/SCons/cppTests.py b/src/engine/SCons/cppTests.py
index 40236e4..1447a7b 100644
--- a/src/engine/SCons/cppTests.py
+++ b/src/engine/SCons/cppTests.py
@@ -26,7 +26,6 @@ from __future__ import absolute_import
 __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
 import atexit
-import sys
 import unittest
 
 import TestUnit
@@ -67,7 +66,7 @@ substitution_input = """
 ifdef_input = """
 #define DEFINED 0
 
-#ifdef	DEFINED
+#ifdef	DEFINED /* multi-line comment */
 #include "file7-yes"
 #else
 #include "file7-no"
@@ -82,7 +81,7 @@ ifdef_input = """
 
 
 if_boolean_input = """
-#define ZERO	0
+#define ZERO	0  // single-line comment
 #define ONE	1
 
 #if ZERO
@@ -132,27 +131,35 @@ if_boolean_input = """
 
 
 if_defined_input = """
-#define DEFINED 0
+#define DEFINED_A 0
+#define DEFINED_B 0
 
-#if	defined(DEFINED)
+#if	defined(DEFINED_A)
 #include "file15-yes"
 #endif
 
-#if	! defined(DEFINED)
+#if	! defined(DEFINED_A)
 #include <file16-no>
 #else
 #include <file16-yes>
 #endif
 
-#if	defined DEFINED
+#if	defined DEFINED_A
 #include "file17-yes"
 #endif
 
-#if	! defined DEFINED
+#if	! defined DEFINED_A
 #include <file18-no>
 #else
 #include <file18-yes>
 #endif
+
+#if ! (defined (DEFINED_A) || defined (DEFINED_B)
+#include <file19-no>
+#else
+#include <file19-yes>
+#endif
+
 """
 
 
@@ -226,11 +233,17 @@ expression_input = """
 #include "file29-yes"
 #endif
 
-#if	! (ONE != ONE)
+#if ! (ONE != ONE)
 #include <file30-yes>
 #else
 #include <file30-no>
 #endif
+
+#if	123456789UL || 0x13L
+#include <file301-yes>
+#else
+#include <file301-no>
+#endif
 """
 
 
@@ -466,6 +479,7 @@ class PreProcessorTestCase(cppAllTestCase):
         ('include', '<', 'file16-yes'),
         ('include', '"', 'file17-yes'),
         ('include', '<', 'file18-yes'),
+        ('include', '<', 'file19-yes'),
     ]
 
     expression_expect = [
@@ -481,6 +495,7 @@ class PreProcessorTestCase(cppAllTestCase):
         ('include', '<', 'file28-yes'),
         ('include', '"', 'file29-yes'),
         ('include', '<', 'file30-yes'),
+        ('include', '<', 'file301-yes'),
     ]
 
     undef_expect = [
@@ -561,6 +576,8 @@ class DumbPreProcessorTestCase(cppAllTestCase):
         ('include', '"', 'file17-yes'),
         ('include', '<', 'file18-no'),
         ('include', '<', 'file18-yes'),
+        ('include', '<', 'file19-no'),
+        ('include', '<', 'file19-yes'),
     ]
 
     expression_expect = [
@@ -588,6 +605,8 @@ class DumbPreProcessorTestCase(cppAllTestCase):
         ('include', '"', 'file29-yes'),
         ('include', '<', 'file30-yes'),
         ('include', '<', 'file30-no'),
+        ('include', '<', 'file301-yes'),
+        ('include', '<', 'file301-no'),
     ]
 
     undef_expect = [