Merge pull request #3256 from ztessler/listcomps-in-funcaction

fix missing code contents that should be included in signature of a f…

4 files changed, 53 insertions, 41 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 49f18a262..a61e1aee0 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -112,6 +112,11 @@ RELEASE 3.1.0.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE
       Specifically, this fixes the test cases use by Configure.CheckCC() which
       would fail when using -Wstrict-prototypes.
 
+  From Zachary Tessler:
+    - Fix calculation of signatures for FunctionActions that contain list (or set,...)
+      comprehensions whose expressions involve constant literals. Those constants had
+      been ignored in signatures, so changing them did not cause targets to be rebuilt.
+
   From Paweł Tomulik:
     - In the testing framework, module TestCommon, fixed must_contain(),
       must_not_contain(), and related methods of TestCommon class to work with
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 78fb7d03b..bff6003eb 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -258,8 +258,7 @@ def _code_contents(code, docstring=None):
     # function. Note that we have to call _object_contents on each
     # constants because the code object of nested functions can
     # show-up among the constants.
-
-    z = [_object_contents(cc) for cc in code.co_consts[1:]]
+    z = [_object_contents(cc) for cc in code.co_consts if cc != docstring]
     contents.extend(b',(')
     contents.extend(bytearray(',', 'utf-8').join(z))
     contents.extend(b')')
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index b9cc00b29..a27f5983b 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -2252,14 +2252,14 @@ class ObjectContentsTestCase(unittest.TestCase):
 
         # Since the python bytecode has per version differences, we need different expected results per version
         expected = {
-            (2, 7): bytearray(b'0, 0, 0, 0,(N.),(),(d\x00\x00GHd\x01\x00S)'),
-            (3, 5): bytearray(b'0, 0, 0, 0,(N.),(print),(e\x00\x00d\x00\x00\x83\x01\x00\x01d\x01\x00S)'),
-            (3, 6): bytearray(b'0, 0, 0, 0,(N.),(print),(e\x00d\x00\x83\x01\x01\x00d\x01S\x00)'),
-            (3, 7): bytearray(b'0, 0, 0, 0,(N.),(print),(e\x00d\x00\x83\x01\x01\x00d\x01S\x00)'),
+            (2, 7): bytearray(b'0, 0, 0, 0,(Hello, World!),(),(d\x00\x00GHd\x01\x00S)'),
+            (3, 5): bytearray(b'0, 0, 0, 0,(Hello, World!),(print),(e\x00\x00d\x00\x00\x83\x01\x00\x01d\x01\x00S)'),
+            (3, 6): bytearray(b'0, 0, 0, 0,(Hello, World!),(print),(e\x00d\x00\x83\x01\x01\x00d\x01S\x00)'),
+            (3, 7): bytearray(b'0, 0, 0, 0,(Hello, World!),(print),(e\x00d\x00\x83\x01\x01\x00d\x01S\x00)'),
         }
 
-        assert c == expected[sys.version_info[:2]], "Got\n" + repr(c) + "\nExpected \n" + "\n" + expected[
-            sys.version_info[:2]]
+        assert c == expected[sys.version_info[:2]], "Got\n" + repr(c) + "\nExpected \n" + "\n" + repr(expected[
+            sys.version_info[:2]])
 
 
 if __name__ == "__main__":
diff --git a/test/Actions/function.py b/test/Actions/function.py
index 5e5066f37..61311af5d 100644
--- a/test/Actions/function.py
+++ b/test/Actions/function.py
@@ -35,7 +35,7 @@ test = TestSCons.TestSCons()
 #
 # Test that the signature of function action includes all the
 # necessary pieces.
-# 
+#
 
 test.write('SConstruct', r"""
 import re
@@ -56,6 +56,7 @@ options.AddVariables(
     ('extracode', 'Extra code for the builder function', ''),
     ('extraarg', 'Extra arg builder function', ''),
     ('nestedfuncexp', 'Expression for the nested function', 'xxx - b'),
+    ('literal_in_listcomp', 'Literal inside list comprehension', '2'),
 )
 
 optEnv = Environment(options=options, tools=[])
@@ -78,6 +79,7 @@ def toto(header='%(header)s', trailer='%(trailer)s'):
         f.write(bytearray(trailer+'\\n','utf-8'))
         f.write(bytearray(str(foo())+'\\n','utf-8'))
         f.write(bytearray(r.match('aaaa').group(1)+'\\n','utf-8'))
+        f.write(bytearray(str(sum([x*%(literal_in_listcomp)s for x in [1,2]]))+'\\n', 'utf-8'))
         %(extracode)s
         try:
            f.write(bytearray(str(xarg),'utf-8')+b'\\n')
@@ -92,7 +94,7 @@ exec( withClosure % optEnv )
 
 genHeaderBld = SCons.Builder.Builder(
     action = SCons.Action.Action(
-        toto(), 
+        toto(),
         'Generating $TARGET',
         varlist=['ENVDEPS']
         ),
@@ -147,57 +149,63 @@ def runtest(arguments, expectedOutFile, expectedRebuild=True, stderr=""):
 # slow buildbot slaves (*cough* Solaris *cough*).
 
 sys.stdout.write('Original build.\n')
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing a docstring should not cause a rebuild.\n')
-runtest('docstring=ThisBuilderDoesXAndY', """Head:0:1:Tail\n18\naaa\n""", False)
-runtest('docstring=SuperBuilder', """Head:0:1:Tail\n18\naaa\n""", False)
-runtest('docstring=', """Head:0:1:Tail\n18\naaa\n""", False)
+runtest('docstring=ThisBuilderDoesXAndY', """Head:0:1:Tail\n18\naaa\n6\n""", False)
+runtest('docstring=SuperBuilder', """Head:0:1:Tail\n18\naaa\n6\n""", False)
+runtest('docstring=', """Head:0:1:Tail\n18\naaa\n6\n""", False)
 
 sys.stdout.write('Changing a variable in the varlist should cause a rebuild.\n')
-runtest('NbDeps=3', """Head:0:1:2:Tail\n18\naaa\n""")
-runtest('NbDeps=4', """Head:0:1:2:3:Tail\n18\naaa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('NbDeps=3', """Head:0:1:2:Tail\n18\naaa\n6\n""")
+runtest('NbDeps=4', """Head:0:1:2:3:Tail\n18\naaa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing the function code should cause a rebuild.\n')
-runtest('extracode=f.write(bytearray("XX\\n","utf-8"))', """Head:0:1:Tail\n18\naaa\nXX\n""")
-runtest('extracode=a=2', """Head:0:1:Tail\n18\naaa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('extracode=f.write(bytearray("XX\\n","utf-8"))', """Head:0:1:Tail\n18\naaa\n6\nXX\n""")
+runtest('extracode=a=2', """Head:0:1:Tail\n18\naaa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing a constant in the function code should cause a rebuild.\n')
-runtest('separator=,', """Head:0,1,Tail\n18\naaa\n""")
-runtest('separator=;', """Head:0;1;Tail\n18\naaa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('separator=,', """Head:0,1,Tail\n18\naaa\n6\n""")
+runtest('separator=;', """Head:0;1;Tail\n18\naaa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing the code of a nested function should cause a rebuild.\n')
-runtest('nestedfuncexp=b-xxx', """Head:0:1:Tail\n-18\naaa\n""")
-runtest('nestedfuncexp=b+xxx', """Head:0:1:Tail\n32\naaa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('nestedfuncexp=b-xxx', """Head:0:1:Tail\n-18\naaa\n6\n""")
+runtest('nestedfuncexp=b+xxx', """Head:0:1:Tail\n32\naaa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Adding an extra argument should cause a rebuild.\n')
-runtest('extraarg=,xarg=2', """Head:0:1:Tail\n18\naaa\n2\n""")
-runtest('extraarg=,xarg=5', """Head:0:1:Tail\n18\naaa\n5\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('extraarg=,xarg=2', """Head:0:1:Tail\n18\naaa\n6\n2\n""")
+runtest('extraarg=,xarg=5', """Head:0:1:Tail\n18\naaa\n6\n5\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing the value of a default argument should cause a rebuild:  a value.\n')
-runtest('b=0', """Head:0:1:Tail\n25\naaa\n""")
-runtest('b=9', """Head:0:1:Tail\n16\naaa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('b=0', """Head:0:1:Tail\n25\naaa\n6\n""")
+runtest('b=9', """Head:0:1:Tail\n16\naaa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing the value of a default argument should cause a rebuild:  an object.\n')
-runtest('regexp=(aaaa)', """Head:0:1:Tail\n18\naaaa\n""")
-runtest('regexp=aa(a+)', """Head:0:1:Tail\n18\naa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('regexp=(aaaa)', """Head:0:1:Tail\n18\naaaa\n6\n""")
+runtest('regexp=aa(a+)', """Head:0:1:Tail\n18\naa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing the value of a closure cell value should cause a rebuild:  a value.\n')
-runtest('closure_cell_value=32', """Head:0:1:Tail\n25\naaa\n""")
-runtest('closure_cell_value=7', """Head:0:1:Tail\n0\naaa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('closure_cell_value=32', """Head:0:1:Tail\n25\naaa\n6\n""")
+runtest('closure_cell_value=7', """Head:0:1:Tail\n0\naaa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
 
 sys.stdout.write('Changing the value of a closure cell value should cause a rebuild:  a default argument.\n')
-runtest('header=MyHeader:', """MyHeader:0:1:Tail\n18\naaa\n""")
-runtest('trailer=MyTrailer', """Head:0:1:MyTrailer\n18\naaa\n""")
-runtest('', """Head:0:1:Tail\n18\naaa\n""")
+runtest('header=MyHeader:', """MyHeader:0:1:Tail\n18\naaa\n6\n""")
+runtest('trailer=MyTrailer', """Head:0:1:MyTrailer\n18\naaa\n6\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
+
+sys.stdout.write('Changing the value of a literal in a list comprehension should cause a rebuild.\n')
+runtest('literal_in_listcomp=3', """Head:0:1:Tail\n18\naaa\n9\n""")
+runtest('literal_in_listcomp=4', """Head:0:1:Tail\n18\naaa\n12\n""")
+runtest('', """Head:0:1:Tail\n18\naaa\n6\n""")
+
 
 test.pass_test()