[llvm-commits] [PATCH 2/6] Use a separate function to translate from dot to ps

[Joerg Sonnenberger]

On Tue, Jul 05, 2011 at 01:32:42PM -0500, Tobias Grosser wrote:
> first of all, this is the existing code and the only thing I did was
> to move it into a separate function such that this code is split
> into smaller more understandable pieces. For this patch I would
> prefer to not touch the existing code to make it very clear that the
> comment "No functional change intended" actually holds.

I don't believe much in shuffling code around and cleaning it up later.

> In respect of replacing this chain of conditions with a switch, As
> stated before, I would prefer to not do this in this commit. I
> attached a patch that could be committed in a subsequent commit.
> This patch also does not change the behaviour, but changes only the
> style. The resulting code will be:
> 
> switch (program) {
> #if HAVE_DOT
>   case GraphProgram::DOT:   prog = sys::Path(LLVM_PATH_DOT);   break;
> #endif
> #if HAVE_FDP
>   case GraphProgram::FDP:   prog = sys::Path(LLVM_PATH_FDP);   break;
> #endif
> #if HAVE_NEATO
>   case GraphProgram::NEATO: prog = sys::Path(LLVM_PATH_NEATO); break;
> #endif
> #if HAVE_TWOPI
>   case GraphProgram::TWOPI: prog = sys::Path(LLVM_PATH_TWOPI); break;
> #endif
> #if HAVE_CIRCO
>   case GraphProgram::CIRCO: prog = sys::Path(LLVM_PATH_CIRCO); break;
> #endif
>   default:
> 
> #if HAVE_DOT
>     prog = sys::Path(LLVM_PATH_DOT);
> #elif HAVE_FDP
>     prog = sys::Path(LLVM_PATH_FDP);
> #elif HAVE_NEATO
>     prog = sys::Path(LLVM_PATH_NEATO);
> #elif HAVE_TWOPI
>     prog = sys::Path(LLVM_PATH_TWOPI);
> #elif HAVE_CIRCO
>     prog = sys::Path(LLVM_PATH_CIRCO);
> #else
> 
>     llvm_unreachable("No graph program available");
> #endif
> 
> }

Go back a step and consider this code from the perspective of building
binary packages. Why do I need to have graphviz installed at configure
time of LLVM? This should just be

switch (program) {
case GraphProgram::DOT: prog = "dot"; break;
case GraphProgram::FDP: prog = "fdp"; break;
...
default:
  llvm_unreachable("Invalid graph program");
}

If really necessary, make it possible to map the program to hard-coded
path names, but I don't think that's very useful. In that case, it would
certainly be much easier to specify the path to dot and use the -K
option.

Joerg