[cfe-commits] Fix for files located in the rood directory (windows)

Aaron Ballman aaron at aaronballman.com
Fri Jun 15 08:25:21 PDT 2012


It makes it more obvious what you're testing for (to me, at least)
because "/" and "\" are pretty for your eyes to confuse when skimming
code.  That being said, I'm fine with committing the patch as-is.

I don't suppose there's a way for us to add a test case for this though...

~Aaron

On Fri, Jun 15, 2012 at 9:59 AM, Nikola Smiljanic <popizdeh at gmail.com> wrote:
> I agree that it's very confusing but having is_posix_separator instead
> of the explicit check wouldn't make it easier to understand. It would
> be clear that it's checking for posix separator, but you'd still need
> to read the commend to understand why. It's all up to you guys, if you
> don't like it I'll rework the patch, otherwise commit at will.
>
> On Fri, Jun 15, 2012 at 4:19 PM, Aaron Ballman <aaron.ballman at gmail.com> wrote:
>> Overall, I'm happy with the fix, but it is a little hard to follow.  I
>> wonder if it makes sense to have llvm:sys::path::is_posix_separator?
>> Something to make it more clear that we only want to strip the
>> trailing slash if it's a POSIX separator and not a DOS separator
>> without relying on reading the comments.  If that seems like overkill,
>> so be it, I'm not too tied to the idea.
>>
>> Thanks for taking care of this!
>>
>> ~Aaron
>>
>> On Fri, Jun 15, 2012 at 8:32 AM, Nikola Smiljanic <popizdeh at gmail.com> wrote:
>>> This patch should fix http://llvm.org/bugs/show_bug.cgi?id=10331
>>>
>>> As you all know Clang can't read files that are located in the root
>>> dir on Windows. This was introduced while trying to fix the fact that
>>> ::stat on windows can't work with paths that end with slash character
>>> (this broke msys users). The solution was to remove the trailing slash
>>> that is detected via llvm::sys::path::is_separator. But this exposed
>>> another shortcoming of ::stat on Windows, it can't accept root dir
>>> path that doesn't end in backslash character.
>>>
>>> The fix is simple, do an explicit check for '/' and only truncate the
>>> path in this case. This should cover both msys and root dir problem.




More information about the cfe-commits mailing list