This is an archive of the discontinued LLVM Phabricator instance.

[GraphWriter] Change GraphWriter to use NodeRef in GraphTraits
ClosedPublic

Authored by timshen on Aug 16 2016, 2:32 PM.

Details

Summary

This is part of the "NodeType* -> NodeRef" migration. Notice that since
GraphWriter prints object address as identity, I added a static_assert on
NodeRef to be a pointer type.

Diff Detail

Event Timeline

timshen updated this revision to Diff 68257.Aug 16 2016, 2:32 PM
timshen retitled this revision from to [GraphWriter] Change GraphWriter to use NodeRef in GraphTraits.
timshen updated this object.
timshen added a reviewer: dblaikie.
timshen added a subscriber: llvm-commits.
dblaikie edited edge metadata.Aug 16 2016, 4:33 PM

what's the printer used for? Would it be best to generalize it (perhaps adding another extension point to the graph traits, so NodeRefs can be printed out - with a default implementation for pointers)?

what's the printer used for? Would it be best to generalize it (perhaps adding another extension point to the graph traits, so NodeRefs can be printed out - with a default implementation for pointers)?

One usage is to print dot files.

I think it's more like that GraphWriter that requires the input nodes to be both fulfilling GraphTraits, and printable, not that GraphTraits has an optional printable requirement - these are two totally orthogonal traits.

dblaikie accepted this revision.Aug 17 2016, 11:03 AM
dblaikie edited edge metadata.

Thanks for the explanation - I think that makes sense, that node printability is an orthogonal concept from the graph traits, hopefully. (the counterexample to this would be if two graphs have the same noderef type but need different printing schemes, but I thin kthat'd only come up if there was a concept of a "graph" distinct from nodes in the graph traits machinery (ie: if you didn't need fat pointers because you already had access to the graph... anyway))

This revision is now accepted and ready to land.Aug 17 2016, 11:03 AM
timshen updated this revision to Diff 68401.Aug 17 2016, 12:57 PM
timshen edited edge metadata.

Updated static_assert error message to illustrate the right thing to do.

This revision was automatically updated to reflect the committed changes.