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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> 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. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
