[llvm] r202225 - Use StringRef in raw_fd_ostream constructor

[Ben Langmuir]

On Feb 26, 2014, at 7:16 AM, Ben Langmuir <blangmuir at apple.com> wrote:

> 
> On Feb 25, 2014, at 9:49 PM, Alp Toker <alp at nuanti.com> wrote:
> 
>> On 26/02/2014 03:21, Ben Langmuir wrote:
>>> Author: benlangmuir
>>> Date: Tue Feb 25 21:21:00 2014
>>> New Revision: 202225
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=202225&view=rev
>>> Log:
>>> Use StringRef in raw_fd_ostream constructor
>>> 
>>> Modified:
>>>    llvm/trunk/include/llvm/Support/raw_ostream.h
>>>    llvm/trunk/lib/Support/raw_ostream.cpp
>>> 
>>> Modified: llvm/trunk/include/llvm/Support/raw_ostream.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/raw_ostream.h?rev=202225&r1=202224&r2=202225&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Support/raw_ostream.h (original)
>>> +++ llvm/trunk/include/llvm/Support/raw_ostream.h Tue Feb 25 21:21:00 2014
>>> @@ -346,7 +346,7 @@ public:
>>>   /// itself to own the file descriptor. In particular, it will close the
>>>   /// file descriptor when it is done (this is necessary to detect
>>>   /// output errors).
>>> -  raw_fd_ostream(const char *Filename, std::string &ErrorInfo,
>>> +  raw_fd_ostream(StringRef Filename, std::string &ErrorInfo,
>>>                  sys::fs::OpenFlags Flags);
>> 
>> Filenames are a special case in LLVM that get passed around as an opaque const char* often without ever needing having their length measured (like the clang driver code I'm looking at right now).
>> 
>> Your change will incur a strlen() upon every ctor call: not a great problem, but also somewhat redundant -- was there a need that triggered this change?
> 
> I intend to make FileEntry in Clang own the storage for its file name, which seemed like a good opportunity to change the FileEntry getName to return a StringRef, particularly because it is currently being used/abused to also check if the FileEntry is initialized.  This was one of the pieces that uses that name, so I thought I’d avoid a string copy before passing it in.
> 
> What if I switch to raw_fd_ostream(const Twine &Filename, …)?  Then if it is null-terminated we don’t need the strlen call.

Actually, given what you just told me, is changing FIleEntry to use StringRef even a good idea? I could potentially just use c_str() to get the pointer now that I have identified all the places that were checking for a null result.

Ben

> 
> Ben
> 
>> 
>> Alp.
>> 
>>>     /// raw_fd_ostream ctor - FD is the file descriptor that this writes to.  If
>>> 
>>> Modified: llvm/trunk/lib/Support/raw_ostream.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/raw_ostream.cpp?rev=202225&r1=202224&r2=202225&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Support/raw_ostream.cpp (original)
>>> +++ llvm/trunk/lib/Support/raw_ostream.cpp Tue Feb 25 21:21:00 2014
>>> @@ -430,16 +430,15 @@ void format_object_base::home() {
>>> /// occurs, information about the error is put into ErrorInfo, and the
>>> /// stream should be immediately destroyed; the string will be empty
>>> /// if no error occurred.
>>> -raw_fd_ostream::raw_fd_ostream(const char *Filename, std::string &ErrorInfo,
>>> +raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::string &ErrorInfo,
>>>                                sys::fs::OpenFlags Flags)
>>>     : Error(false), UseAtomicWrites(false), pos(0) {
>>> -  assert(Filename != 0 && "Filename is null");
>>>   ErrorInfo.clear();
>>>     // Handle "-" as stdout. Note that when we do this, we consider ourself
>>>   // the owner of stdout. This means that we can do things like close the
>>>   // file descriptor when we're done and set the "binary" flag globally.
>>> -  if (Filename[0] == '-' && Filename[1] == 0) {
>>> +  if (Filename == "-") {
>>>     FD = STDOUT_FILENO;
>>>     // If user requested binary then put stdout into binary mode if
>>>     // possible.
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> -- 
>> http://www.nuanti.com
>> the browser experts