There are now several copies of the code for viewing a dot graph with dot&gv/dotty/Graphviz. This should all be shared in one place in libsupport. -Chris
Specifically, see all the files that implementing PR798 had to change :(
We need to check to make sure all the usages are the same and if not make sure the shared code can handle the variant usages via arguments to the function.
All files actually use WriteGraph() call to output graph to stream. I'll refactor code.
Created attachment 336 [details] Code cleanup Moved graph printing to Support/GraphWriter.h, fixed the correspondent calls.
Anton, The patch looks fine, except: I don't like having ViewGraph implementation in a header file. This is going to get replicated in all those other files which doesn't buy us anything for code size. I understand the need to put it in a .h because of the template. But, the template doesn't use GraphType& G except in the call to WriteGraph. So, could you please refactor this so that the common parts are put in separate functions in a .cpp file. Here's what I'd like to see: 1. Make an overload of WriteGraph that takes the same arguments as the ViewGraph function (GraphType, Name, Title). This would create its own temporary file and return that file name as its result (sys::Path). This is the first part of your patch (up to the #ifdef). 2. Create a new function, DisplayGraph, which takes a sys::Path that is presumed to be the postscript file to view. This function incorporates the second part of your patch (everything after #ifdef). 3. Reduce the ViewGraph template to just: template<typename GraphType> void ViewGraph(const GraphType& G, const std::string& Name, const std::string& Title = "") { sys::Path path = WriteGraph(G,Name,Title) if (path.empty()) return; DisplayGraph(path) } Reid.
Another cleanup that might be required before this can be done: we should make the GraphWriter interface not be a big template with traits to allow customization. Instead we should use inheritance, and subclassing to customize writing. This would allow the huge template to come out of the header, and would reduce code size. -Chris
Created attachment 342 [details] Refactored code Reid, this patch implements the functionality you've mentioned.
Mine.
The patch did not apply correctly. In particular, lib/Analysis/DataStructure/Pringer.cpp failed. It looks like this code has changed since the patch. Could you update to CVS head, fix any merge/conflict problems and then provide a new patch. I'll try to apply it as soon as possible to avoid this problem. Thanks, Reid.
Anton, Any updates on this bug? Reid.
Reid! As you know already, I'm in the process of migration to new system. I'm managing to start working on this issue on monday.
Created attachment 352 [details] Patch over latest CVS snapshot Reid, this is the updated patch. Hope it will apply fine.
Created attachment 353 [details] cvs diff -u output Due to unknown reason new file lib/Support/GraphWriter.cpp wasn't included to cvs diff output ("-N" switch was used). I'll upload it separately.
Created attachment 354 [details] cvs diff -u output
Created attachment 355 [details] missed GraphWriter.cpp
The new patches applied correctly. I'm building and checking to make sure it breaks nothing. If that works, I'll commit the patches. Thanks, Anton.
Bug Resolved. Relevant Patches: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060626/035737.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060626/035738.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060626/035739.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060626/035740.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060626/035741.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060626/035742.html