[FX] Fix python code having spurious newlines from placeholders

[jamesr66a]

Stack from ghstack:

• [FX] Fix python code having spurious newlines from placeholders #49720 [FX] Fix python code having spurious newlines from placeholders

The logic in Graph.python_code didn't account for placeholder nodes with name == target not emitting a line, so it was spuriously emitting one newline per placeholder in the Graph. This patch fixes that.

Test script:

import torch
import torch.fx

graph = torch.fx.Graph()
for i in range(30):
    ph_value = graph.placeholder(f'v{i}')
graph.output(ph_value)

print(graph.python_code(''))

Before

def forward(self, v0, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16, v17, v18, v19, v20, v21, v22, v23, v24, v25, v26, v27, v28, v29):
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    return v29

After

def forward(self, v0, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16, v17, v18, v19, v20, v21, v22, v23, v24, v25, v26, v27, v28, v29):
    return v29

Differential Revision: D25675825

[facebook-github-bot]

💊 CI failures summary and remediations

As of commit cca698a (more details on the Dr. CI page):

---

💚 💚 Looks good so far! There are no failures yet. 💚 💚

---

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 3 times.

[zdevito]

Can this be refactored? The newline handling can just be:

emit_node(node)
delete_unused_values(node)
body.append('\n')

[jamesr66a]

@zdevito that would still incorrectly append the newline after a placeholder with name == target, right?

[facebook-github-bot]

@jamesr66a merged this pull request in 11598da.

[facebook-github-bot]

@jamesr66a merged this pull request in 11598da.

[zdevito]

The newline handling here is still pretty convoluted now -- there shouldn't need to be comment saying that some unrelated function is handling a newline for another function. Is it possible to refactor this so that newline handling for a statement occurs in one place?