amccarth marked an inline comment as done. amccarth added a comment. Thanks for the review.
In D69958#1744861 <https://reviews.llvm.org/D69958#1744861>, @vsapsai wrote: > There is one issue with a comment, apart of that everything looks good to me. > Looked into replacing slash and backslash literals with calls to > `llvm::sys::path::get_separator` or `llvm::sys::path::is_separator` but in my > attempts failed to improve existing patch. Yes, I originally started down the path of trying to "normalize" path styles (like slash direction), but that always led to collateral damage because we literally have situations where a path is a hybrid of both Windows and Posix. Rather than writing Windows and Posix versions of the tests, this patch takes the approach that choosing a path style is out-of-scope, which I'm told is Clang's general strategy. ================ Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:535 /// All configuration options are optional. /// 'case-sensitive': <boolean, default=true> /// 'use-external-names': <boolean, default=true> ---------------- vsapsai wrote: > I believe changing default `CaseSensitive` value makes this comment > incorrect. Have no strong opinion on making case-insensitive default on > Windows because a better requirement is for VFS generators to specify > 'case-sensitive' explicitly instead of relying on default values. Make CaseSensitive system-dependent aligns with UseCanonicalizePaths, so this feels pretty consistent. (Perhaps in a future path, we can fix both of those.) In the mean time, I'll update the comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69958/new/ https://reviews.llvm.org/D69958 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
