This is an archive of the discontinued LLVM Phabricator instance.

[Dominators] Make applyUpdate's documentation less confusing [NFC]
ClosedPublic

Authored by kuhar on Jul 27 2018, 3:20 PM.

Details

Summary

It was pointed out by @chandlerc that it's not clear whether both applyUpdates and insert/deleteEdge can be used to perform multiple updates.

IMO, the confusing part was that the comment above applyUpdates made a comparison of expected update time between calling it and calling insert/deleteEdge multiple times. It's generally not possible to safely call insert/deleteEdge multiple times, which documentation for each of the 3 functions warns about, so the whole comparison makes very little sense. On top of that, the comment is already lengthy, so I think it's best to just get rid of this comparison.

Diff Detail

Repository
rL LLVM

Event Timeline

kuhar created this revision.Jul 27 2018, 3:20 PM
kuhar retitled this revision from [Dominators] Make applyUpdate's documentation less confusing to [Dominators] Make applyUpdate's documentation less confusing [NFC].
chandlerc accepted this revision.Jul 27 2018, 5:51 PM

LGTM! Thanks for the improved comment. Hopefully at least helps future me when I get confused reading it again.

This revision is now accepted and ready to land.Jul 27 2018, 5:51 PM
This revision was automatically updated to reflect the committed changes.