Skip to content

Commit

Permalink
Merge pull request #3256 from ztessler/listcomps-in-funcaction
Browse files Browse the repository at this point in the history
fix missing code contents that should be included in signature of a f…
  • Loading branch information
bdbaddog committed Dec 25, 2018
2 parents 4daeb49 + f707964 commit 2e684c2
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 41 deletions.
5 changes: 5 additions & 0 deletions src/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/engine/SCons/Action.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')')
Expand Down
12 changes: 6 additions & 6 deletions src/engine/SCons/ActionTests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2252,14 +2252,14 @@ def test_code_contents(self):

# 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__":
Expand Down
74 changes: 41 additions & 33 deletions test/Actions/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#
# Test that the signature of function action includes all the
# necessary pieces.
#
#

test.write('SConstruct', r"""
import re
Expand All @@ -56,6 +56,7 @@
('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=[])
Expand All @@ -78,6 +79,7 @@ def foo(b=b):
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')
Expand All @@ -92,7 +94,7 @@ def foo(b=b):
genHeaderBld = SCons.Builder.Builder(
action = SCons.Action.Action(
toto(),
toto(),
'Generating $TARGET',
varlist=['ENVDEPS']
),
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 2e684c2

Please sign in to comment.