[llvm-commits] [PATCH] New Path API design.

[Chris Lattner]

On Nov 20, 2010, at 3:06 PM, Michael Spencer wrote:

>> +struct file_type {
>> +  enum _ {
>> 
>> Why _ instead of giving it a name?  I'd suggest file_type_kind.  We use "kind" for discriminators elsewhere.
> 
> The enums are like this to emulate 'enum class' from C++0x. Users
> never directly use the _ name, they use 'file_type::xyz'.

Ok.

>> +  int v_;
>> 
>> Likewise, why v_? Particularly if this is public data it should have a good name and a doxygen comment.
> 
> It should have been private. This is again just a helper to emulate
> 'enum class'.

Ok.

>> +/// file_status - Represents the result of a call to stat and friends. It has
>> +///               a platform specific member to store the result.
>> +class file_status
>> +{
>> 
>> Seems fine so far.  Clang in particular is highly performance sensitive when it comes to stats and wants to reduce them wherever possible.  It really wants to do things like use "open + fstat" instead of "stat+open" since fstat is faster than stat.  This isn't a problem with your proposal, just pointing it out so that you're aware.
> 
> Interesting. It should be simple enough to add a function that returns
> both a raw_ostream and a file_status.

Yep, I imagine that you'll find it when stuff starts converting over.  Just be conscientious about syscalls in clang :)
 

>> +error_code make_absolute(SmallVectorImpl<char> &path);
>> 
>> Please add doxygen comments to these with an example.  Also, what happens if an empty string is passed in?
> 
> I'm going to fully document everything. An empty string would result
> in the current directory.

Thanks again Michael!

-Chris