SMAT-Lab / Scalpel

Scalpel: The Python Static Analysis Framework
Apache License 2.0
286 stars 43 forks source link

Constant propagation for loops with counter does not work #66

Closed simisimon closed 2 years ago

simisimon commented 2 years ago

I think this problem is related to #37. However, I believe this here is more complex.

Consider the following code snippet:

num_points = 300
C_range = np.geomspace(start=1e-7, stop=1e7, num=num_points)

for i, C_ in enumerate(C_range):
    lr_l2_C = LogisticRegression(penalty = 'l2', solver='liblinear', C=C_)

With Scalpel, I'm currently not able to get the values for the variables C_. Do you plan to handle such cases as well?

simisimon commented 2 years ago

Running the code below, prints the following:`.

    code_str = """
    C_range = np.geomspace(start=1e-7, stop=1e7)

    for i, C_ in enumerate(C_range):
        lr_l2_C = LogisticRegression(penalty = 'l2', solver='liblinear', C=C_)
    """  

    cfg = CFGBuilder().build_from_src(name="", src=code_str)

    _, const_dict = SSA().compute_SSA(cfg)

    print(const_dict)
{('C_range', 0): <ast.Call object at 0x00000235E61DCEE0>, ('i', 0): None, ('C_', 0): None, ('lr_l2_C', 0): <ast.Call object at 0x00000235E61DCA30>}
('C_range', 0) np.geomspace(start=1e-07, stop=10000000.0)

i and not C_ have no value. Would it be possible to assign the call of enumerate to them, as we did in #37?

Jarvx commented 2 years ago

Running the code below, prints the following:`.

    code_str = """
    C_range = np.geomspace(start=1e-7, stop=1e7)

    for i, C_ in enumerate(C_range):
        lr_l2_C = LogisticRegression(penalty = 'l2', solver='liblinear', C=C_)
    """  

    cfg = CFGBuilder().build_from_src(name="", src=code_str)

    _, const_dict = SSA().compute_SSA(cfg)

    print(const_dict)
{('C_range', 0): <ast.Call object at 0x00000235E61DCEE0>, ('i', 0): None, ('C_', 0): None, ('lr_l2_C', 0): <ast.Call object at 0x00000235E61DCA30>}
('C_range', 0) np.geomspace(start=1e-07, stop=10000000.0)

i and not C_ have no value. Would it be possible to assign the call of enumerate to them, as we did in #37?

Thanks for reporting this. This is reasonable and has been suggested by another developer. We will update this.

simisimon commented 2 years ago

I also created a PR for this issue here and added a corresponding test.