Document the non-local invariants in code: pdn ordering and obstruction rules, and the same pattern in tap and ant

While reading src/pdn to understand how a grid is assembled, I ran into several behaviours that are load-bearing but not stated anywhere near the code that depends on them. Each took a while to pin down, because in every case the reason lives in a different file from the effect.

I would like to suggest short comments at the point of reliance, rather than a separate design document. A design document drifts away from the code; a comment on the line someone is editing does not.

Four places in pdn where a sentence would have saved a lot of reading.

1. Component order is insertion order, and -connect_to_pads inserts early

Grid::getGridComponents() concatenates rings_ and then straps_, and straps_ is in the order components were added. Because -connect_to_pads is an argument to define_pdn_grid, its straps enter straps_ before any add_pdn_stripe for that grid — so pad direct connections are built immediately after the rings and before every strap.

Nothing in the pad code says this, and it matters: a pad connection's getClosestShape can only see the rings.

2. GridComponent::make is a four-step contract

makeShapes(shapes);
cutShapes(obstructions);
getObstructions(obstructions);
getShapes(shapes);

Each component is cut against everything built before it and then becomes an obstruction for everything after. The consequence — that moving one component changes its neighbours' geometry — is not visible from any single subclass.

3. Shape::cut's same-net exemption turns on the obstruction's type

if (other_shape->net_ != nullptr && net_ == other_shape->net_
    && other_shape->shapeType() != ShapeType::kShape) {

The != kShape condition is what makes the exemption apply to pins and never to another component's shapes. That is only apparent once you have also read GridComponent::getObstructions, which inserts a component's shapes into the obstruction tree unchanged so they stay kShape, and getInstanceObstructions, which builds pin shapes with a net and then flips them to kBlockObs.

Two functions in other files decide whether that condition can ever be true. A comment at either end pointing at the other would make the pairing visible.

4. The deferred retry is not defensive

for (auto* component : getGridComponents())
  if (!component->make(local_shapes, local_obstructions)) deferred.push_back(component);
for (auto* component : deferred)
  component->make(local_shapes, local_obstructions);

This reads as a safety net, but at least one configuration depends on it: a grid with -connect_to_pads and no ring has nothing for the pad connections to reach when they run, so they build nothing, defer, and connect on the retry once the straps exist.

The debug output is doing a lot of this already

set_debug_level PDN Make 3 and set_debug_level PDN Shape 3 document a great deal of the above executably — component boundaries in order, the shape count either side of every cut, and each shape as it is added. That was more informative than anything I read, and it is the reason I can describe the four items above with any confidence.

Two small things would help people find it: a pointer to the available groups from the module README or from PdnGen's header, and the group names listed somewhere greppable. At present you have to find the debugPrint call sites to learn that Make, Shape, Straps, Via, ViaEnclosure and the rest exist.

The same pattern shows up in other modules

This is not really a pdn issue — it is the shape of question that comes up wherever the reason for a behaviour is one or two files away from the behaviour. Two examples I hit elsewhere:

tap. Tapcell::run() is cutRows → placeEndcaps → placeTapcells, and the order is load-bearing in the same way as above: endcaps occupy sites before findValidLocation and isOverlapping go looking. Separately, placeTapcells opens with

const bool disallow_one_site_gaps = !odb::hasOneSiteMaster(db_);

so the gap rule applied inside the placement loop depends on whether any master anywhere in the database is one site wide. Reading placeTapcells alone will not tell you that a library change elsewhere alters its output.

ant. In calculatePAR, whether a node uses diff_metal_factor or metal_factor — and whether the getAreaDiffReduce PWL is consulted at all — turns on info.iterm_diff_area != 0, a quantity accumulated while walking the net somewhere else entirely. The two branches are different formulas, not a correction term.

Both of those are perfectly reasonable designs. The point is only that a reader cannot discover the dependency from the function they are standing in, which is exactly when an inline comment earns its keep.

It is also worth noting how uneven the debug coverage is: pdn defines sixteen debug groups, tap defines two, and ant defines none. Given how much the pdn groups helped, extending that practice looks like it would pay off well beyond documentation alone.

Offer

Happy to open a PR adding the comments if you agree on placement and wording — or to leave it with you if you would rather write them yourselves. I am equally happy for this to be closed as "working as intended" if the view is that reading the surrounding files is the expected cost.