[PATCH] fix for CGDebugInfo.cpp filename append

[David Blaikie]

On Thu, Oct 17, 2013 at 12:23 PM, Yaron Keren <yaron.keren at gmail.com> wrote:

> Yes, using .str() will work, but it involves creating yet another
> temporary, a StringRef
>

Constructing a StringRef is cheap.


> returned from .str() and then converted to std::string.
>

Conversion/return from std::string should invoke RVO, at least. That just
leaves the copy in the assignment which in C++11 would be move assignment.
We only really care about performance of LLVM and Clang in a C++11 build
anyway, so I'm OK with relying on that for performance (though move
assignment isn't quite as efficient as the assign call, since it can't
reuse the string's existing buffer - but since the string was just default
constructed it won't have any existing buffer (on any well-performing
implementation) to begin with).


> An optimizer may help here.
> The most efficient code would be
>
>       MainFileName.assign(MainFileDirSS.data(), MainFileDirSS.size());
>
> What I was hoping for is to avoid using the SmallString conversions.
>
> Are the path functions using SmallStrings for efficiency or is there
> another reason?
>

Since they need to mutate the argument they have to choose some concrete
string type and SmallString is generally preferred in the LLVM codebase. It
might be easier to just change MainFileName to SmallString if that's
practical (unless some other caller needs it in a std::string)


>
> Yaron
>
>
> 2013/10/17 David Blaikie <dblaikie at gmail.com>
>
>> I think instead of writing:
>>
>> MainFileName = std::string(MainFileDirSS.data(), MainFileDirSS.size());
>>
>> you can just write:
>>
>> MainFileName = MainFileDirSS.str();
>>
>> Hopefully.
>
>
>
> On Thu, Oct 17, 2013 at 8:09 AM, Yaron Keren <yaron.keren at gmail.com>wrote:
>
>> Hi,
>>
>> Filename appending in CGDebugInfo.cpp was implemented as simple string
>> concatenation, resulting in duplicate file debug information on Windows due
>> to wrong path separator.
>>
>> The patch uses  llvm::sys::path::append to properly append the filename
>> on all OS.
>>
>> Is there a way to use path::append without the SmallString temporary?
>>
>> Yaron
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131017/892b8e39/attachment.html>