[cfe-dev] Are there plans for a libDriver?

[Ted Kremenek]

On Aug 5, 2008, at 11:40 AM, Nico Weber wrote:

> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080805/3ffdb038/attachment.html>