Are there plans for a libDriver?

Hi,

so I've been playing around with using clang as a library the last few days. Here's an early program I wrote: C++ code - 236 lines - codepad

The program is intended to be a drop-in replacement for gcc, so that it can be used like `CC=myprogram make`. The -I and -D options of gcc are processed, the rest of the options are silently ignored. All the program does is to list all global variables defined in the current translation unit.

Even though the program can parse really complex C, it is only 236 lines thanks to clang. However, it still could be a lot shorter.

28 lines of the program define a diagnostic client. I use a simplified (and less useful) version of TextDiagnosticPrinter. I'd be happy to use TextDiagnosticPrinter directly, but it is defined in Driver, so I can't use it.

35 lines define a function that can add directories to the include path (addIncludePath()) or that can define built-in macros (DefineBuiltinMacro()). Both functions are stolen from the Driver directory (DefineBuiltinMacro() is actually defined twice in clang: once in clang.cpp, once in Preprocessor.cpp, both times as a static function. Both versions are identical -- so there's obviously a need for this function in other programs :wink: ). Again, functions like this should already be available.

Creating a Preprocessor object takes 14 lines in this program. I need to create a DiagnosticsClient object, a Diagnostics object, a LangOptions object, a TargetInfo, a SourceManager, FileManager, and a HeaderSearch before I can create a Preprocessor. There should be a (say) PreprocessorContext that contains all those classes and that uses reasonable defaults for everything, so that creating a Preprocessor with friends is just one or two lines.

Creating a Preprocessor object is in my eyes the "hello world" of clang, so this should be simpler.

Another 12 lines are used for adding the default header search paths. The PreprocessorContext (or whatever) should already add those include directories.

Are there plans for a libDriver, which makes writing simple, "one-off" clang clients easier? With this library, the amount of code of this program could be reduced by a whole order of magnitude (only the ASTConsumer, two short loops for the -D and -I options, and the call to ParseAST would be left).

Thanks a lot for clang :slight_smile:

Nico

Hi Nico,

You are not alone. There is a lot of interest in doing this, but nobody has gotten around to doing it yet. If you are interested, feel free to drive some work on this by submitting patches, etc.

Best,
Ted

The attached patch adds directories and Makefiles for a libDriver and moves the TextDiagnostic classes over there for starters. I hope `svn diff` encodes `svn mv` information correctly.

Nico

libdriver.patch (23 KB)

In this case, I moved the options to clang.cpp and added flags to TextDiagnosticPrinter, so there are no options in libDriver now.

Nico

libdriver.patch (26.5 KB)

Instead of setShowColumn, etc., it seems better just to set these values in the ctor of TextDiagnosticPrinter. That would transform:

       TextDiagnosticPrinter* TextDiagPrinter = new TextDiagnosticPrinter;
       TextDiagPrinter->setShowColumn(!NoShowColumn);
       TextDiagPrinter->setShowCaretDiagnostics(!NoCaretDiagnostics);

into:

       TextDiagnosticPrinter* TextDiagPrinter = new TextDiagnosticPrinter(NoShowColumn, NoCaretDiagnostics);

It doesn't seem likely that carets/columns would be switched on/off after the printer is constructed.

Instead of setShowColumn, etc., it seems better just to set these values in the ctor of TextDiagnosticPrinter. That would transform:

     TextDiagnosticPrinter* TextDiagPrinter = new TextDiagnosticPrinter;
     TextDiagPrinter->setShowColumn(!NoShowColumn);
     TextDiagPrinter->setShowCaretDiagnostics(!NoCaretDiagnostics);

into:

     TextDiagnosticPrinter* TextDiagPrinter = new TextDiagnosticPrinter(NoShowColumn, NoCaretDiagnostics);

It doesn't seem likely that carets/columns would be switched on/off after the printer is constructed.

I thought about that, but I don't like bools in constructors. It's ok if they come from a variable, but when I see code like

   TextDiagnosticPrinter printer(true, false);

then I wonder what those bools do. Most clients will use the default parameters anyways.

Furthermore, TDP has an error stream as parameter that defaults to cerr. Should that be first or last? Both uses seem equally likely, so there should probably at least two constructors.

But if this is the clang way, I won't object. Updated patch (with just this change) is attached.

Nico

libdriver.patch (26 KB)

I thought about that, but I don’t like bools in constructors. It’s ok if they come from a variable, but when I see code like

TextDiagnosticPrinter printer(true, false);

then I wonder what those bools do. Most clients will use the default parameters anyways.

I agree there are tradeoffs. The good thing about putting the bools into the ctor is that:

(a) a constructed TextDiagnosticPrinter object is complete once its constructed. While not so important here, it can be very useful for ensuring correctness in the code as an object is not created in an incomplete state.

(b) the code is more succinct

In general, the problem you describe doesn’t just apply to bools, but also to places like where you can pass NULL as a parameter value:

foo(…, NULL, NULL)

C++ doesn’t have labeled function/method calls, so at some point this is just an inherit limitation in the language.

Another strategy is to use enums instead of bools, e.g:

TextDiagnosticPrinter* p =
new TextDiagnosticPrinter( TextDiagnosticPrinter::DisplayCarrots, TextDiagnosticPrinter::DisplayColumns, …)

This is more verbose, but harnesses type checking to make sure people pass the right options (in this example DisplayCarrots and DisplayColumns would be enum constants from two separate enum declarations within TextDiagnosticPrinter).

Furthermore, TDP has an error stream as parameter that defaults to cerr. Should that be first or last? Both uses seem equally likely, so there should probably at least two constructors.

The advantage of putting the options for displaying the columns/carrots first is that people have to thing about the form of the output they want when using a different output steam.

But if this is the clang way, I won’t object. Updated patch (with just this change) is attached.

Nuno just applied a patch that fixed a bug in TextDiagnosticPrinter:

http://llvm.org/viewvc/llvm-project?rev=54365&view=rev

Please include those changes in your patch (if you haven’t already) and apply the patch.

Thanks Nico!

Ted

Hello,

Nico Weber wrote:
[SNIP]

But if this is the clang way, I won't object. Updated patch (with just this change) is attached.

My attached patch updates the MSVC project to use the new path for the TextDiagnostics files that went into r54383.

text_diag_reorg.diff (1.2 KB)

Hello,

Nico Weber wrote:
[SNIP]

But if this is the clang way, I won't object. Updated patch (with just this change) is attached.

My attached patch updates the MSVC project to use the new path for the
TextDiagnostics files that went into r54383.

text_diag_reorg.diff (1.2 KB)

We need to create a new VCProject for libDriver, not update the paths for Driver itself. I'll try and get a patch working after lunch.

The VS project/solution files have been updated. Please report any problems.

TextDiagnosticPrinter::DisplayCarrots

V. important for extending to other vegetables. :slight_smile: